SECURITY: Add FinalDestination::FastImage that's SSRF safe

This commit is contained in:
Ted Johansson 2023-02-16 12:02:03 +08:00 committed by Blake Erickson
parent 6dcb099547
commit 39c2f63b35
4 changed files with 95 additions and 2 deletions

View File

@ -193,7 +193,7 @@ module CookedProcessorMixin
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.

View File

@ -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

View File

@ -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

View File

@ -220,7 +220,7 @@ RSpec.describe SearchIndexer 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,
)