CVE-2024-37157: Insufficient Information (NVD-CWE-noinfo)
value: Discourse is an open-source discussion platform. Prior to version 3.2.3 on the `stable` branch and version 3.3.0.beta4 on the `beta` and `tests-passed` branches, a malicious actor could get the FastImage library to redirect requests to an internal Discourse IP. This issue is patched in version 3.2.3 on the `stable` branch and version 3.3.0.beta4 on the `beta` and `tests-passed` branches. No known workarounds are available.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-07-03T20:15Z
Repository: https://github.com/discourse/discourse
Language: Ruby
Files changed: lib/cooked_processor_mixin.rb, lib/freedom_patches/fast_image.rb, spec/lib/freedom_patches/fast_image_spec.rb, spec/services/search_indexer_spec.rb
Commit date: 2024-07-03 20:49:15 +0800
Commit message: SECURITY: Add FastImage SSRF safe freedom patch

Diff: diff --git a/lib/cooked_processor_mixin.rb b/lib/cooked_processor_mixin.rb
index fcf9df667c9d0..b31d27b6c9f72 100644
--- a/lib/cooked_processor_mixin.rb
+++ b/lib/cooked_processor_mixin.rb
@@ -191,11 +191,11 @@ module CookedProcessorMixin
 
     upload = Upload.get_from_url(absolute_url)
     if upload && upload.width && upload.width > 0
       @size_cache[url] = [upload.width, upload.height]
     else
-      @size_cache[url] = FinalDestination::FastImage.size(absolute_url)
+      @size_cache[url] = FastImage.size(absolute_url)
     end
   rescue Zlib::BufError, URI::Error, OpenSSL::SSL::SSLError
     # FastImage.size raises BufError for some gifs, leave it.
   end
 
diff --git a/lib/final_destination/fast_image.rb b/lib/final_destination/fast_image.rb
deleted file mode 100644
index 615c7848db42c..0000000000000
--- a/lib/final_destination/fast_image.rb
+++ /dev/null
@@ -1,23 +0,0 @@
-# frozen_string_literal: true
-
-class FinalDestination::FastImage < ::FastImage
-  def initialize(url, options = {})
-    uri = URI(normalized_url(url))
-    options.merge!(http_header: { "Host" => uri.hostname })
-    uri.hostname = resolved_ip(uri)
-
-    super(uri.to_s, options)
-  rescue FinalDestination::SSRFDetector::DisallowedIpError, SocketError, Timeout::Error
-    super("")
-  end
-
-  private
-
-  def resolved_ip(uri)
-    FinalDestination::SSRFDetector.lookup_and_filter_ips(uri.hostname).first
-  end
-
-  def normalized_url(uri)
-    UrlHelper.normalized_encode(uri)
-  end
-end
diff --git a/lib/freedom_patches/fast_image.rb b/lib/freedom_patches/fast_image.rb
new file mode 100644
index 0000000000000..68be1f5afca63
--- /dev/null
+++ b/lib/freedom_patches/fast_image.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class FastImage
+  def setup_http
+    @http = FinalDestination::HTTP.new(@parsed_uri.host, @parsed_uri.port)
+    @http.use_ssl = (@parsed_uri.scheme == "https")
+    @http.verify_mode = OpenSSL::SSL::VERIFY_NONE
+    @http.open_timeout = @options[:timeout]
+    @http.read_timeout = @options[:timeout]
+  end
+end
diff --git a/spec/lib/final_destination/fast_image_spec.rb b/spec/lib/final_destination/fast_image_spec.rb
deleted file mode 100644
index b04129ec60b64..0000000000000
--- a/spec/lib/final_destination/fast_image_spec.rb
+++ /dev/null
@@ -1,70 +0,0 @@
-# frozen_string_literal: true
-
-describe FinalDestination::FastImage do
-  before do
-    # We need to test low-level stuff, switch off WebMock for FastImage
-    WebMock.enable!(except: [:net_http])
-    Socket.stubs(:tcp).never
-    TCPSocket.stubs(:open).never
-    Addrinfo.stubs(:getaddrinfo).never
-  end
-
-  after { WebMock.enable! }
-
-  def expect_tcp_and_abort(stub_addr, &blk)
-    success = Class.new(StandardError)
-    TCPSocket.stubs(:open).with { |addr| stub_addr == addr }.once.raises(success)
-    begin
-      yield
-    rescue success
-    end
-  end
-
-  def stub_ip_lookup(stub_addr, ips)
-    FinalDestination::SSRFDetector.stubs(:lookup_ips).with { |addr| stub_addr == addr }.returns(ips)
-  end
-
-  def stub_tcp_to_raise(stub_addr, exception)
-    TCPSocket.stubs(:open).with { |addr| addr == stub_addr }.once.raises(exception)
-  end
-
-  it "uses the first resolved IP" do
-    stub_ip_lookup("example.com", %w[1.1.1.1 2.2.2.2 3.3.3.3])
-    expect_tcp_and_abort("1.1.1.1") do
-      FinalDestination::FastImage.size(URI("https://example.com/img.jpg"))
-    end
-  end
-
-  it "ignores private IPs" do
-    stub_ip_lookup("example.com", %w[0.0.0.0 2.2.2.2])
-    expect_tcp_and_abort("2.2.2.2") do
-      FinalDestination::FastImage.size(URI("https://example.com/img.jpg"))
-    end
-  end
-
-  it "returns a null object when all IPs are private" do
-    stub_ip_lookup("example.com", %w[0.0.0.0 127.0.0.1])
-    expect(FinalDestination::FastImage.size(URI("https://example.com/img.jpg"))).to eq(nil)
-  end
-
-  it "returns a null object if all IPs are blocked" do
-    SiteSetting.blocked_ip_blocks = "98.0.0.0/8|78.13.47.0/24|9001:82f3::/32"
-    stub_ip_lookup("ip6.example.com", %w[9001:82f3:8873::3])
-    stub_ip_lookup("ip4.example.com", %w[98.23.19.111])
-    expect(FinalDestination::FastImage.size(URI("https://ip4.example.com/img.jpg"))).to eq(nil)
-    expect(FinalDestination::FastImage.size(URI("https://ip6.example.com/img.jpg"))).to eq(nil)
-  end
-
-  it "allows specified hosts to bypass IP checks" do
-    SiteSetting.blocked_ip_blocks = "98.0.0.0/8|78.13.47.0/24|9001:82f3::/32"
-    SiteSetting.allowed_internal_hosts = "internal.example.com|blocked-ip.example.com"
-    stub_ip_lookup("internal.example.com", %w[0.0.0.0 127.0.0.1])
-    stub_ip_lookup("blocked-ip.example.com", %w[98.23.19.111])
-    expect_tcp_and_abort("0.0.0.0") do
-      FinalDestination::FastImage.size(URI("https://internal.example.com/img.jpg"))
-    end
-    expect_tcp_and_abort("98.23.19.111") do
-      FinalDestination::FastImage.size(URI("https://blocked-ip.example.com/img.jpg"))
-    end
-  end
-end
diff --git a/spec/lib/freedom_patches/fast_image_spec.rb b/spec/lib/freedom_patches/fast_image_spec.rb
new file mode 100644
index 0000000000000..32bb2a1aceaad
--- /dev/null
+++ b/spec/lib/freedom_patches/fast_image_spec.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+RSpec.describe FastImage do
+  before do
+    FinalDestination::SSRFDetector.allow_ip_lookups_in_test!
+    WebMock.enable!(except: [:final_destination])
+  end
+
+  after do
+    WebMock.enable!
+    FinalDestination::SSRFDetector.disallow_ip_lookups_in_test!
+  end
+
+  it "should filter endpoint hostname through our SSRF detector and return null object" do
+    stub_ip_lookup("example.com", %W[0.0.0.0])
+
+    expect(described_class.type("http://example.com")).to eq(nil)
+  end
+
+  it "should send the right request if endpoint hostname resolves to a public ip address" do
+    stub_ip_lookup("example.com", %W[52.125.123.12])
+
+    success = Class.new(StandardError)
+    TCPSocket.stubs(:open).with { |addr| "52.125.123.12" == addr }.once.raises(success)
+
+    expect { described_class.type("http://example.com") }.to raise_error(success)
+  end
+end
diff --git a/spec/services/search_indexer_spec.rb b/spec/services/search_indexer_spec.rb
index f784fcd83cbcd..599c98d00886b 100644
--- a/spec/services/search_indexer_spec.rb
+++ b/spec/services/search_indexer_spec.rb
@@ -236,17 +236,14 @@ RSpec.describe SearchIndexer do
 
     it "should not include lightbox in search" do
       Jobs.run_immediately!
       SiteSetting.max_image_width = 1
 
-      stub_request(:get, "https://1.2.3.4/some.png").to_return(
-        status: 200,
-        body: file_from_fixtures("logo.png").read,
-      )
-
       src = "https://meta.discourse.org/some.png"
 
+      stub_request(:get, src).to_return(status: 200, body: file_from_fixtures("logo.png").read)
+
       post = Fabricate(:post, raw: <<~RAW)
       Let me see how I can fix this image
       <img src="#{src}" title="GOT" alt="white walkers" width="2" height="2">
       RAW
 

Vulnerable code:       @size_cache[url] = FinalDestination::FastImage.size(absolute_url)
# frozen_string_literal: true

class FinalDestination::FastImage < ::FastImage
  def initialize(url, options = {})
    uri = URI(normalized_url(url))
    options.merge!(http_header: { "Host" => uri.hostname })
    uri.hostname = resolved_ip(uri)

    super(uri.to_s, options)
  rescue FinalDestination::SSRFDetector::DisallowedIpError, SocketError, Timeout::Error
    super("")
  end

  private

  def resolved_ip(uri)
    FinalDestination::SSRFDetector.lookup_and_filter_ips(uri.hostname).first
  end

  def normalized_url(uri)
    UrlHelper.normalized_encode(uri)
  end
end
# frozen_string_literal: true

describe FinalDestination::FastImage do
  before do
    # We need to test low-level stuff, switch off WebMock for FastImage
    WebMock.enable!(except: [:net_http])
    Socket.stubs(:tcp).never
    TCPSocket.stubs(:open).never
    Addrinfo.stubs(:getaddrinfo).never
  end

  after { WebMock.enable! }

  def expect_tcp_and_abort(stub_addr, &blk)
    success = Class.new(StandardError)
    TCPSocket.stubs(:open).with { |addr| stub_addr == addr }.once.raises(success)
    begin
      yield
    rescue success
    end
  end

  def stub_ip_lookup(stub_addr, ips)
    FinalDestination::SSRFDetector.stubs(:lookup_ips).with { |addr| stub_addr == addr }.returns(ips)
  end

  def stub_tcp_to_raise(stub_addr, exception)
    TCPSocket.stubs(:open).with { |addr| addr == stub_addr }.once.raises(exception)
  end

  it "uses the first resolved IP" do
    stub_ip_lookup("example.com", %w[1.1.1.1 2.2.2.2 3.3.3.3])
    expect_tcp_and_abort("1.1.1.1") do
      FinalDestination::FastImage.size(URI("https://example.com/img.jpg"))
    end
  end

  it "ignores private IPs" do
    stub_ip_lookup("example.com", %w[0.0.0.0 2.2.2.2])
    expect_tcp_and_abort("2.2.2.2") do
      FinalDestination::FastImage.size(URI("https://example.com/img.jpg"))
    end
  end

  it "returns a null object when all IPs are private" do
    stub_ip_lookup("example.com", %w[0.0.0.0 127.0.0.1])
    expect(FinalDestination::FastImage.size(URI("https://example.com/img.jpg"))).to eq(nil)
  end

  it "returns a null object if all IPs are blocked" do
    SiteSetting.blocked_ip_blocks = "98.0.0.0/8|78.13.47.0/24|9001:82f3::/32"
    stub_ip_lookup("ip6.example.com", %w[9001:82f3:8873::3])
    stub_ip_lookup("ip4.example.com", %w[98.23.19.111])
    expect(FinalDestination::FastImage.size(URI("https://ip4.example.com/img.jpg"))).to eq(nil)
    expect(FinalDestination::FastImage.size(URI("https://ip6.example.com/img.jpg"))).to eq(nil)
  end

  it "allows specified hosts to bypass IP checks" do
    SiteSetting.blocked_ip_blocks = "98.0.0.0/8|78.13.47.0/24|9001:82f3::/32"
    SiteSetting.allowed_internal_hosts = "internal.example.com|blocked-ip.example.com"
    stub_ip_lookup("internal.example.com", %w[0.0.0.0 127.0.0.1])
    stub_ip_lookup("blocked-ip.example.com", %w[98.23.19.111])
    expect_tcp_and_abort("0.0.0.0") do
      FinalDestination::FastImage.size(URI("https://internal.example.com/img.jpg"))
    end
    expect_tcp_and_abort("98.23.19.111") do
      FinalDestination::FastImage.size(URI("https://blocked-ip.example.com/img.jpg"))
    end
  end
end
      stub_request(:get, "https://1.2.3.4/some.png").to_return(
        status: 200,
        body: file_from_fixtures("logo.png").read,
      )

Fixed code:       @size_cache[url] = FastImage.size(absolute_url)
# frozen_string_literal: true

class FastImage
  def setup_http
    @http = FinalDestination::HTTP.new(@parsed_uri.host, @parsed_uri.port)
    @http.use_ssl = (@parsed_uri.scheme == "https")
    @http.verify_mode = OpenSSL::SSL::VERIFY_NONE
    @http.open_timeout = @options[:timeout]
    @http.read_timeout = @options[:timeout]
  end
end
# frozen_string_literal: true

RSpec.describe FastImage do
  before do
    FinalDestination::SSRFDetector.allow_ip_lookups_in_test!
    WebMock.enable!(except: [:final_destination])
  end

  after do
    WebMock.enable!
    FinalDestination::SSRFDetector.disallow_ip_lookups_in_test!
  end

  it "should filter endpoint hostname through our SSRF detector and return null object" do
    stub_ip_lookup("example.com", %W[0.0.0.0])

    expect(described_class.type("http://example.com")).to eq(nil)
  end

  it "should send the right request if endpoint hostname resolves to a public ip address" do
    stub_ip_lookup("example.com", %W[52.125.123.12])

    success = Class.new(StandardError)
    TCPSocket.stubs(:open).with { |addr| "52.125.123.12" == addr }.once.raises(success)

    expect { described_class.type("http://example.com") }.to raise_error(success)
  end
end
      stub_request(:get, src).to_return(status: 200, body: file_from_fixtures("logo.png").read)