SECURITY: Prevent Onebox cache overflow by limiting downloads and URL lengths

lib/onebox.rb

@@ -16,7 +16,7 @@ module Onebox
   DEFAULTS = {
     connect_timeout: 5,
     timeout: 10,
-    max_download_kb: (10 * 1024), # 10MB
+    max_download_kb: 2048, # 2MB
     load_paths: [File.join(Rails.root, "lib/onebox/templates")],
     allowed_ports: [80, 443],
     allowed_schemes: %w[http https],

lib/onebox/engine/standard_embed.rb

@@ -100,7 +100,13 @@ module Onebox
           ).first
         favicon = favicon.nil? ? nil : (favicon["href"].nil? ? nil : favicon["href"].strip)
 
-        Onebox::Helpers.get_absolute_image_url(favicon, url)
+        return nil if favicon.blank?
+
+        absolute_url = Onebox::Helpers.get_absolute_image_url(favicon, url)
+
+        return nil if absolute_url.length > UrlHelper::MAX_URL_LENGTH
+
+        absolute_url
       end
 
       def get_description

lib/url_helper.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class UrlHelper
-  MAX_URL_LENGTH = 100_000
+  MAX_URL_LENGTH = 2_000
 
   # At the moment this handles invalid URLs that browser address bar accepts
   # where second # is not encoded

spec/lib/onebox/engine/standard_embed_spec.rb

@@ -66,6 +66,34 @@ RSpec.describe Onebox::Engine::StandardEmbed do
       expect(instance.raw).to eq({ title: "do not override me" })
     end
 
+    it "sets favicon URL" do
+      html_doc =
+        mocked_html_doc(
+          twitter_data: {
+            "name" => "twitter:url",
+            "content" => "cool.url",
+          },
+          favicon_url: "https://favicon.co/default.ico",
+        )
+      Onebox::Helpers.stubs(fetch_html_doc: html_doc)
+
+      expect(instance.raw).to eq({ url: "cool.url", favicon: "https://favicon.co/default.ico" })
+    end
+
+    it "ignores suspiciously long favicon URLs" do
+      html_doc =
+        mocked_html_doc(
+          twitter_data: {
+            "name" => "twitter:url",
+            "content" => "cool.url",
+          },
+          favicon_url: "https://favicon.co/#{"a" * 2_000}.ico",
+        )
+      Onebox::Helpers.stubs(fetch_html_doc: html_doc)
+
+      expect(instance.raw).to eq({ url: "cool.url" })
+    end
+
     it "sets oembed data" do
       Onebox::Helpers.stubs(fetch_html_doc: nil)
       Onebox::Oembed.any_instance.stubs(:data).returns({ description: "description" })
@@ -84,11 +112,11 @@ RSpec.describe Onebox::Engine::StandardEmbed do
 
   private
 
-  def mocked_html_doc(twitter_data: nil)
+  def mocked_html_doc(twitter_data: nil, favicon_url: nil)
     html_doc = mock
     html_doc.stubs(at_css: nil, at: nil)
     stub_twitter(html_doc, twitter_data)
-    stub_favicon(html_doc)
+    stub_favicon(html_doc, favicon_url)
     stub_json_ld
     html_doc
   end
@@ -97,13 +125,13 @@ RSpec.describe Onebox::Engine::StandardEmbed do
     html_doc.expects(:css).with("meta").at_least_once.returns([twitter_data])
   end
 
-  def stub_favicon(html_doc)
+  def stub_favicon(html_doc, favicon_url = nil)
     html_doc
       .stubs(:css)
       .with(
         'link[rel="shortcut icon"], link[rel="icon shortcut"], link[rel="shortcut"], link[rel="icon"]',
       )
-      .returns([])
+      .returns([{ "href" => favicon_url }.compact])
   end
 
   def stub_json_ld

spec/lib/url_helper_spec.rb

@@ -166,7 +166,7 @@ RSpec.describe UrlHelper do
     end
 
     it "raises error if too long" do
-      expect do UrlHelper.normalized_encode("https://#{"a" * 100_000}.com") end.to raise_error(
+      expect do UrlHelper.normalized_encode("https://#{"a" * 2_000}.com") end.to raise_error(
         ArgumentError,
       )
     end