SECURITY: Add FastImage SSRF safe freedom patch
This commit is contained in:
parent
89c0123b3a
commit
67e7808603
|
@ -193,7 +193,7 @@ module CookedProcessorMixin
|
|||
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.
|
||||
|
|
|
@ -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
|
|
@ -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
|
|
@ -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
|
|
@ -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
|
|
@ -238,13 +238,10 @@ RSpec.describe SearchIndexer 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">
|
||||
|
|
Loading…
Reference in New Issue