CVE-2023-28112: Server-Side Request Forgery (SSRF) (CWE-918)
value: Discourse is an open-source discussion platform. Prior to version 3.1.0.beta3 of the `beta` and `tests-passed` branches, some user provided URLs were being passed to FastImage without SSRF protection. Insufficient protections could enable attackers to trigger outbound network connections from the Discourse server to private IP addresses. This affects any site running the `tests-passed` or `beta` branches versions 3.1.0.beta2 and prior. This issue is patched in version 3.1.0.beta3 of the `beta` and `tests-passed` branches. There are no known workarounds.
CWE description: The web server receives a URL or similar request from an upstream component and retrieves the contents of this URL, but it does not sufficiently ensure that the request is being sent to the expected destination.
CVSS v3 base score: 8.1
Published: 2023-03-17T19:15Z
Repository: https://github.com/discourse/discourse
Language: Ruby
Files changed: lib/cooked_processor_mixin.rb, lib/final_destination/fast_image.rb, spec/lib/final_destination/fast_image_spec.rb, spec/services/search_indexer_spec.rb
Commit date: 2023-03-16 15:27:09 -0600
Commit message: SECURITY: Add FinalDestination::FastImage that's SSRF safe

Diff: diff --git a/lib/cooked_processor_mixin.rb b/lib/cooked_processor_mixin.rb
index 4947a9a02eea..1ad4d74853ae 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] = FastImage.size(absolute_url)
+      @size_cache[url] = FinalDestination::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
new file mode 100644
index 000000000000..615c7848db42
--- /dev/null
+++ b/lib/final_destination/fast_image.rb
@@ -0,0 +1,23 @@
+# 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/spec/lib/final_destination/fast_image_spec.rb b/spec/lib/final_destination/fast_image_spec.rb
new file mode 100644
index 000000000000..b04129ec60b6
--- /dev/null
+++ b/spec/lib/final_destination/fast_image_spec.rb
@@ -0,0 +1,70 @@
+# 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/services/search_indexer_spec.rb b/spec/services/search_indexer_spec.rb
index 9e1b9a887018..13890c884c03 100644
--- a/spec/services/search_indexer_spec.rb
+++ b/spec/services/search_indexer_spec.rb
@@ -218,11 +218,11 @@ RSpec.describe SearchIndexer do
 
     it "should not include lightbox in search" do
       Jobs.run_immediately!
       SiteSetting.max_image_width = 1
 
-      stub_request(:get, "https://meta.discourse.org/some.png").to_return(
+      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"

Vulnerable code:       @size_cache[url] = FastImage.size(absolute_url)
      stub_request(:get, "https://meta.discourse.org/some.png").to_return(
Fixed 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(