Commit f363c92a authored by GitLab Release Tools Bot's avatar GitLab Release Tools Bot

Merge branch 'security-dns-ssrf-bypass-11-11' into '11-11-stable'

Server Side Request Forgery mitigation bypass

See merge request gitlab/gitlabhq!3214
parents 1d4a1300 de9b7a69
---
title: Fix Server Side Request Forgery mitigation bypass
merge_request:
author:
type: security
......@@ -20,6 +20,7 @@ module Gitlab
# Returns an array with [<uri>, <original-hostname>].
# rubocop:disable Metrics/CyclomaticComplexity
# rubocop:disable Metrics/ParameterLists
# rubocop:disable Metrics/PerceivedComplexity
def validate!(
url,
ports: [],
......@@ -32,6 +33,7 @@ module Gitlab
dns_rebind_protection: true)
# rubocop:enable Metrics/CyclomaticComplexity
# rubocop:enable Metrics/ParameterLists
# rubocop:enable Metrics/PerceivedComplexity
return [nil, nil] if url.nil?
......@@ -56,7 +58,15 @@ module Gitlab
addr.ipv6_v4mapped? ? addr.ipv6_to_ipv4 : addr
end
rescue SocketError
return [uri, nil]
# In the test suite we use a lot of mocked urls that are either invalid or
# don't exist. In order to avoid modifying a ton of tests and factories
# we allow invalid urls unless the environment variable RSPEC_ALLOW_INVALID_URLS
# is not true
return [uri, nil] if Rails.env.test? && ENV['RSPEC_ALLOW_INVALID_URLS'] == 'true'
# If the addr can't be resolved or the url is invalid (i.e http://1.1.1.1.1)
# we block the url
raise BlockedUrlError, "Host cannot be resolved or invalid"
end
protected_uri_with_hostname = enforce_uri_hostname(addrs_info, uri, hostname, dns_rebind_protection)
......@@ -92,9 +102,9 @@ module Gitlab
# we'll be making the request to the IP address, instead of using the hostname.
def enforce_uri_hostname(addrs_info, uri, hostname, dns_rebind_protection)
address = addrs_info.first
ip_address = address&.ip_address
ip_address = address.ip_address
return [uri, nil] unless dns_rebind_protection && ip_address && ip_address != hostname
return [uri, nil] unless dns_rebind_protection && ip_address != hostname
uri = uri.dup
uri.hostname = ip_address
......
......@@ -68,6 +68,16 @@ describe Gitlab::UrlBlocker do
expect(uri).to eq(Addressable::URI.parse('https://example.org'))
expect(hostname).to eq(nil)
end
context 'when it cannot be resolved' do
let(:import_url) { 'http://foobar.x' }
it 'raises error' do
stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
expect { described_class.validate!(import_url) }.to raise_error(described_class::BlockedUrlError)
end
end
end
context 'when the URL hostname is an IP address' do
......@@ -79,6 +89,16 @@ describe Gitlab::UrlBlocker do
expect(uri).to eq(Addressable::URI.parse('https://93.184.216.34'))
expect(hostname).to be(nil)
end
context 'when it is invalid' do
let(:import_url) { 'http://1.1.1.1.1' }
it 'raises an error' do
stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
expect { described_class.validate!(import_url) }.to raise_error(described_class::BlockedUrlError)
end
end
end
end
end
......@@ -180,8 +200,6 @@ describe Gitlab::UrlBlocker do
end
it 'returns true for a non-alphanumeric hostname' do
stub_resolv
aggregate_failures do
expect(described_class).to be_blocked_url('ssh://-oProxyCommand=whoami/a')
......@@ -300,10 +318,6 @@ describe Gitlab::UrlBlocker do
end
context 'when enforce_user is' do
before do
stub_resolv
end
context 'false (default)' do
it 'does not block urls with a non-alphanumeric username' do
expect(described_class).not_to be_blocked_url('ssh://-oProxyCommand=whoami@example.com/a')
......@@ -351,6 +365,18 @@ describe Gitlab::UrlBlocker do
expect(described_class.blocked_url?('https://git‌lab.com/foo/foo.bar', ascii_only: true)).to be true
end
end
it 'blocks urls with invalid ip address' do
stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
expect(described_class).to be_blocked_url('http://8.8.8.8.8')
end
it 'blocks urls whose hostname cannot be resolved' do
stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
expect(described_class).to be_blocked_url('http://foobar.x')
end
end
describe '#validate_hostname!' do
......@@ -382,10 +408,4 @@ describe Gitlab::UrlBlocker do
end
end
end
# Resolv does not support resolving UTF-8 domain names
# See https://bugs.ruby-lang.org/issues/4270
def stub_resolv
allow(Resolv).to receive(:getaddresses).and_return([])
end
end
......@@ -3,6 +3,7 @@ SimpleCovEnv.start!
ENV["RAILS_ENV"] = 'test'
ENV["IN_MEMORY_APPLICATION_SETTINGS"] = 'true'
ENV["RSPEC_ALLOW_INVALID_URLS"] = 'true'
require File.expand_path('../config/environment', __dir__)
require 'rspec/rails'
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment