SECURITY: Use FinalDestination for topic embeds

app/models/topic_embed.rb

@@ -109,7 +109,14 @@ class TopicEmbed < ActiveRecord::Base
 
     url = UrlHelper.escape_uri(url)
     original_uri = URI.parse(url)
-    raise URI::InvalidURIError unless original_uri.is_a?(URI::HTTP)
+    fd = FinalDestination.new(
+      url,
+      validate_uri: true,
+      max_redirects: 5
+    )
+
+    url = fd.resolve
+    raise URI::InvalidURIError if url.blank?
 
     opts = {
       tags: %w[div p code pre h1 h2 h3 b em i strong a img ul li ol blockquote],

spec/models/topic_embed_spec.rb

@@ -166,6 +166,7 @@ describe TopicEmbed do
       before do
         file.stubs(:read).returns contents
         TopicEmbed.stubs(:open).returns file
+        stub_request(:head, url)
       end
 
       it "doesn't scrub the title by default" do
@@ -194,6 +195,7 @@ describe TopicEmbed do
         SiteSetting.embed_classname_whitelist = 'emoji, foo'
         file.stubs(:read).returns contents
         TopicEmbed.stubs(:open).returns file
+        stub_request(:head, url)
         response = TopicEmbed.find_remote(url)
       end
 
@@ -230,6 +232,7 @@ describe TopicEmbed do
       before(:each) do
         file.stubs(:read).returns contents
         TopicEmbed.stubs(:open).returns file
+        stub_request(:head, url)
         response = TopicEmbed.find_remote(url)
       end
 
@@ -252,6 +255,7 @@ describe TopicEmbed do
         SiteSetting.embed_classname_whitelist = ''
         file.stubs(:read).returns contents
         TopicEmbed.stubs(:open).returns file
+        stub_request(:head, url)
         response = TopicEmbed.find_remote(url)
       end
 
@@ -279,9 +283,8 @@ describe TopicEmbed do
       let!(:file) { StringIO.new }
 
       before do
-        file.stubs(:read).returns contents
+        stub_request(:head, url)
-        TopicEmbed.stubs(:open)
+        stub_request(:get, url).to_return(body: contents).then.to_raise
-          .with('http://eviltrout.com/test/%D9%85%D8%A7%D9%87%DB%8C', allow_redirections: :safe).returns file
       end
 
       it "doesn't throw an error" do
@@ -297,9 +300,8 @@ describe TopicEmbed do
       let!(:file) { StringIO.new }
 
       before do
-        file.stubs(:read).returns contents
+        stub_request(:head, url)
-        TopicEmbed.stubs(:open)
+        stub_request(:get, url).to_return(body: contents).then.to_raise
-          .with('http://example.com/hello%20world', allow_redirections: :safe).returns file
       end
 
       it "doesn't throw an error" do
@@ -310,7 +312,6 @@ describe TopicEmbed do
 
     context "non-http URL" do
       let(:url) { '/test.txt' }
-
       it "throws an error" do
         expect { TopicEmbed.find_remote(url) }.to raise_error(URI::InvalidURIError)
       end
@@ -328,6 +329,7 @@ describe TopicEmbed do
       end
 
       it "handles mailto links" do
+        stub_request(:head, url)
         response = TopicEmbed.find_remote(url)
         expect(response.body).to have_tag('a', with: { href: 'mailto:foo%40example.com' })
         expect(response.body).to have_tag('a', with: { href: 'mailto:bar@example.com' })