From a9f5b22394954be8941566da1cf349bb6a179974 Mon Sep 17 00:00:00 2001 From: Steve Azzopardi Date: Mon, 26 Nov 2018 08:45:38 +0000 Subject: [PATCH] Merge branch 'security-11-5-fix-webhook-ssrf-ipv6' into 'security-11-5' [11.5] Fix SSRF in project integrations See merge request gitlab/gitlabhq!2611 --- .../security-fix-webhook-ssrf-ipv6.yml | 5 + lib/gitlab/url_blocker.rb | 12 +- spec/lib/gitlab/url_blocker_spec.rb | 108 ++++++++++++++++-- 3 files changed, 112 insertions(+), 13 deletions(-) create mode 100644 changelogs/unreleased/security-fix-webhook-ssrf-ipv6.yml diff --git a/changelogs/unreleased/security-fix-webhook-ssrf-ipv6.yml b/changelogs/unreleased/security-fix-webhook-ssrf-ipv6.yml new file mode 100644 index 00000000000..32c85a2a7da --- /dev/null +++ b/changelogs/unreleased/security-fix-webhook-ssrf-ipv6.yml @@ -0,0 +1,5 @@ +--- +title: Fix SSRF in project integrations +merge_request: +author: +type: security diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb index fa401abc6bb..b8040f73cee 100644 --- a/lib/gitlab/url_blocker.rb +++ b/lib/gitlab/url_blocker.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'resolv' +require 'ipaddress' module Gitlab class UrlBlocker @@ -23,7 +24,9 @@ module Gitlab validate_hostname!(uri.hostname) begin - addrs_info = Addrinfo.getaddrinfo(uri.hostname, port, nil, :STREAM) + addrs_info = Addrinfo.getaddrinfo(uri.hostname, port, nil, :STREAM).map do |addr| + addr.ipv6_v4mapped? ? addr.ipv6_to_ipv4 : addr + end rescue SocketError return true end @@ -82,13 +85,14 @@ module Gitlab def validate_hostname!(value) return if value.blank? + return if IPAddress.valid?(value) return if value =~ /\A\p{Alnum}/ - raise BlockedUrlError, "Hostname needs to start with an alphanumeric character" + raise BlockedUrlError, "Hostname or IP address invalid" end def validate_localhost!(addrs_info) - local_ips = ["127.0.0.1", "::1", "0.0.0.0"] + local_ips = ["::", "0.0.0.0"] local_ips.concat(Socket.ip_address_list.map(&:ip_address)) return if (local_ips & addrs_info.map(&:ip_address)).empty? @@ -103,7 +107,7 @@ module Gitlab end def validate_local_network!(addrs_info) - return unless addrs_info.any? { |addr| addr.ipv4_private? || addr.ipv6_sitelocal? } + return unless addrs_info.any? { |addr| addr.ipv4_private? || addr.ipv6_sitelocal? || addr.ipv6_unique_local? } raise BlockedUrlError, "Requests to the local network are not allowed" end diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb index 35b550283b5..39e0a17a307 100644 --- a/spec/lib/gitlab/url_blocker_spec.rb +++ b/spec/lib/gitlab/url_blocker_spec.rb @@ -38,23 +38,37 @@ describe Gitlab::UrlBlocker do end it 'returns true for localhost IPs' do + expect(described_class.blocked_url?('https://[0:0:0:0:0:0:0:0]/foo/foo.git')).to be true expect(described_class.blocked_url?('https://0.0.0.0/foo/foo.git')).to be true - expect(described_class.blocked_url?('https://[::1]/foo/foo.git')).to be true - expect(described_class.blocked_url?('https://127.0.0.1/foo/foo.git')).to be true + expect(described_class.blocked_url?('https://[::]/foo/foo.git')).to be true end it 'returns true for loopback IP' do expect(described_class.blocked_url?('https://127.0.0.2/foo/foo.git')).to be true + expect(described_class.blocked_url?('https://127.0.0.1/foo/foo.git')).to be true + expect(described_class.blocked_url?('https://[::1]/foo/foo.git')).to be true end it 'returns true for alternative version of 127.0.0.1 (0177.1)' do expect(described_class.blocked_url?('https://0177.1:65535/foo/foo.git')).to be true end + it 'returns true for alternative version of 127.0.0.1 (017700000001)' do + expect(described_class.blocked_url?('https://017700000001:65535/foo/foo.git')).to be true + end + it 'returns true for alternative version of 127.0.0.1 (0x7f.1)' do expect(described_class.blocked_url?('https://0x7f.1:65535/foo/foo.git')).to be true end + it 'returns true for alternative version of 127.0.0.1 (0x7f.0.0.1)' do + expect(described_class.blocked_url?('https://0x7f.0.0.1:65535/foo/foo.git')).to be true + end + + it 'returns true for alternative version of 127.0.0.1 (0x7f000001)' do + expect(described_class.blocked_url?('https://0x7f000001:65535/foo/foo.git')).to be true + end + it 'returns true for alternative version of 127.0.0.1 (2130706433)' do expect(described_class.blocked_url?('https://2130706433:65535/foo/foo.git')).to be true end @@ -63,6 +77,27 @@ describe Gitlab::UrlBlocker do expect(described_class.blocked_url?('https://127.000.000.001:65535/foo/foo.git')).to be true end + it 'returns true for alternative version of 127.0.0.1 (127.0.1)' do + expect(described_class.blocked_url?('https://127.0.1:65535/foo/foo.git')).to be true + end + + context 'with ipv6 mapped address' do + it 'returns true for localhost IPs' do + expect(described_class.blocked_url?('https://[0:0:0:0:0:ffff:0.0.0.0]/foo/foo.git')).to be true + expect(described_class.blocked_url?('https://[::ffff:0.0.0.0]/foo/foo.git')).to be true + expect(described_class.blocked_url?('https://[::ffff:0:0]/foo/foo.git')).to be true + end + + it 'returns true for loopback IPs' do + expect(described_class.blocked_url?('https://[0:0:0:0:0:ffff:127.0.0.1]/foo/foo.git')).to be true + expect(described_class.blocked_url?('https://[::ffff:127.0.0.1]/foo/foo.git')).to be true + expect(described_class.blocked_url?('https://[::ffff:7f00:1]/foo/foo.git')).to be true + expect(described_class.blocked_url?('https://[0:0:0:0:0:ffff:127.0.0.2]/foo/foo.git')).to be true + expect(described_class.blocked_url?('https://[::ffff:127.0.0.2]/foo/foo.git')).to be true + expect(described_class.blocked_url?('https://[::ffff:7f00:2]/foo/foo.git')).to be true + end + end + it 'returns true for a non-alphanumeric hostname' do stub_resolv @@ -86,7 +121,22 @@ describe Gitlab::UrlBlocker do end context 'when allow_local_network is' do - let(:local_ips) { ['192.168.1.2', '10.0.0.2', '172.16.0.2'] } + let(:local_ips) do + [ + '192.168.1.2', + '[0:0:0:0:0:ffff:192.168.1.2]', + '[::ffff:c0a8:102]', + '10.0.0.2', + '[0:0:0:0:0:ffff:10.0.0.2]', + '[::ffff:a00:2]', + '172.16.0.2', + '[0:0:0:0:0:ffff:172.16.0.2]', + '[::ffff:ac10:20]', + '[feef::1]', + '[fee2::]', + '[fc00:bf8b:e62c:abcd:abcd:aaaa:aaaa:aaaa]' + ] + end let(:fake_domain) { 'www.fakedomain.fake' } context 'true (default)' do @@ -117,10 +167,14 @@ describe Gitlab::UrlBlocker do expect(described_class).not_to be_blocked_url('http://169.254.168.100') end - # This is blocked due to the hostname check: https://gitlab.com/gitlab-org/gitlab-ce/issues/50227 - it 'blocks IPv6 link-local endpoints' do - expect(described_class).to be_blocked_url('http://[::ffff:169.254.169.254]') - expect(described_class).to be_blocked_url('http://[::ffff:169.254.168.100]') + it 'allows IPv6 link-local endpoints' do + expect(described_class).not_to be_blocked_url('http://[0:0:0:0:0:ffff:169.254.169.254]') + expect(described_class).not_to be_blocked_url('http://[::ffff:169.254.169.254]') + expect(described_class).not_to be_blocked_url('http://[::ffff:a9fe:a9fe]') + expect(described_class).not_to be_blocked_url('http://[0:0:0:0:0:ffff:169.254.168.100]') + expect(described_class).not_to be_blocked_url('http://[::ffff:169.254.168.100]') + expect(described_class).not_to be_blocked_url('http://[::ffff:a9fe:a864]') + expect(described_class).not_to be_blocked_url('http://[fe80::c800:eff:fe74:8]') end end @@ -143,14 +197,20 @@ describe Gitlab::UrlBlocker do end it 'blocks IPv6 link-local endpoints' do + expect(described_class).to be_blocked_url('http://[0:0:0:0:0:ffff:169.254.169.254]', allow_local_network: false) expect(described_class).to be_blocked_url('http://[::ffff:169.254.169.254]', allow_local_network: false) + expect(described_class).to be_blocked_url('http://[::ffff:a9fe:a9fe]', allow_local_network: false) + expect(described_class).to be_blocked_url('http://[0:0:0:0:0:ffff:169.254.168.100]', allow_local_network: false) expect(described_class).to be_blocked_url('http://[::ffff:169.254.168.100]', allow_local_network: false) - expect(described_class).to be_blocked_url('http://[FE80::C800:EFF:FE74:8]', allow_local_network: false) + expect(described_class).to be_blocked_url('http://[::ffff:a9fe:a864]', allow_local_network: false) + expect(described_class).to be_blocked_url('http://[fe80::c800:eff:fe74:8]', allow_local_network: false) end end def stub_domain_resolv(domain, ip) - allow(Addrinfo).to receive(:getaddrinfo).with(domain, any_args).and_return([double(ip_address: ip, ipv4_private?: true, ipv6_link_local?: false, ipv4_loopback?: false, ipv6_loopback?: false)]) + address = double(ip_address: ip, ipv4_private?: true, ipv6_link_local?: false, ipv4_loopback?: false, ipv6_loopback?: false) + allow(Addrinfo).to receive(:getaddrinfo).with(domain, any_args).and_return([address]) + allow(address).to receive(:ipv6_v4mapped?).and_return(false) end def unstub_domain_resolv @@ -191,6 +251,36 @@ describe Gitlab::UrlBlocker do end end + describe '#validate_hostname!' do + let(:ip_addresses) do + [ + '2001:db8:1f70::999:de8:7648:6e8', + 'FE80::C800:EFF:FE74:8', + '::ffff:127.0.0.1', + '::ffff:169.254.168.100', + '::ffff:7f00:1', + '0:0:0:0:0:ffff:0.0.0.0', + 'localhost', + '127.0.0.1', + '127.000.000.001', + '0x7f000001', + '0x7f.0.0.1', + '0x7f.0.0.1', + '017700000001', + '0177.1', + '2130706433', + '::', + '::1' + ] + end + + it 'does not raise error for valid Ip addresses' do + ip_addresses.each do |ip| + expect { described_class.send(:validate_hostname!, ip) }.not_to raise_error + end + end + end + # Resolv does not support resolving UTF-8 domain names # See https://bugs.ruby-lang.org/issues/4270 def stub_resolv