summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFrancisco Javier López <fjlopez@gitlab.com>2018-11-12 11:44:33 +0100
committerFrancisco Javier López <fjlopez@gitlab.com>2018-11-12 11:44:33 +0100
commitdcf8c62f64afc7632f62a7179e8f8827190f4dc1 (patch)
treec77d17e6836edf59857f004bbf9b37fa2dc06a1d
parent1c315f4c26ee0d682dd232c077a1bf38a7634b70 (diff)
downloadgitlab-ce-dcf8c62f64afc7632f62a7179e8f8827190f4dc1.tar.gz
Fix SSRF in project integrations
This commit fixes a SSRF vulnerability related to project hooks and ipv6 addresses. It also addresses a problem with ipv6 mapped addresses.
-rw-r--r--changelogs/unreleased/security-fix-webhook-ssrf-ipv6.yml5
-rw-r--r--lib/gitlab/url_blocker.rb12
-rw-r--r--spec/lib/gitlab/url_blocker_spec.rb108
3 files changed, 112 insertions, 13 deletions
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 a2cce9151c3..ab3071dff53 100644
--- a/lib/gitlab/url_blocker.rb
+++ b/lib/gitlab/url_blocker.rb
@@ -1,4 +1,5 @@
require 'resolv'
+require 'ipaddress'
module Gitlab
class UrlBlocker
@@ -24,7 +25,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
@@ -71,13 +74,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?
@@ -92,7 +96,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 8df0facdab3..48524578562 100644
--- a/spec/lib/gitlab/url_blocker_spec.rb
+++ b/spec/lib/gitlab/url_blocker_spec.rb
@@ -30,23 +30,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
@@ -55,6 +69,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
@@ -78,7 +113,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
@@ -109,10 +159,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
@@ -135,14 +189,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
@@ -183,6 +243,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