FIX: only redirect to whitelisted hostnames

app/models/topic_link_click.rb

@@ -13,6 +13,8 @@ class TopicLinkClick < ActiveRecord::Base
   validates_presence_of :topic_link_id
   validates_presence_of :ip_address
 
+  WHITELISTED_REDIRECT_HOSTNAMES = Set.new(%W{www.youtube.com youtu.be})
+
   # Create a click from a URL and post_id
   def self.create_from(args={})
     url = args[:url]
@@ -52,7 +54,10 @@ class TopicLinkClick < ActiveRecord::Base
       # If we have it somewhere else on the site, just allow the redirect.
       # This is likely due to a onebox of another topic.
       link = TopicLink.find_by(url: url)
-      return link.present? ? link.url : url
+      return link.url if link.present?
+
+      # Only redirect to whitelisted hostnames
+      return WHITELISTED_REDIRECT_HOSTNAMES.include?(uri.hostname) ? url : nil
     end
 
     return url if args[:user_id] && link.user_id == args[:user_id]

spec/models/topic_link_click_spec.rb

@@ -45,10 +45,15 @@ describe TopicLinkClick do
     context 'create_from' do
 
       context 'without a url' do
-        let(:click) { TopicLinkClick.create_from(url: "url that doesn't exist", post_id: @post.id, ip: '127.0.0.1') }
 
-        it "returns the url" do
-          expect(click).to eq("url that doesn't exist")
+        it "returns nil to prevent redirect exploit" do
+          click = TopicLinkClick.create_from(url: "http://url-that-doesnt-exist.com", post_id: @post.id, ip: '127.0.0.1')
+          expect(click).to eq(nil)
+        end
+
+        it "redirect to whitelisted hostnames" do
+          click = TopicLinkClick.create_from(url: "https://www.youtube.com/watch?v=jYd_5aggzd4", post_id: @post.id, ip: '127.0.0.1')
+          expect(click).to eq("https://www.youtube.com/watch?v=jYd_5aggzd4")
         end
       end