Commit c0ab38a3 authored by Mike Greiling's avatar Mike Greiling Committed by Mike Greiling

Merge branch 'rs-alphanumeric-ssh-params-8-17' into 'security-8-17'

Ensure user and hostnames begin with an alnum character in UrlBlocker

See merge request !2153
parent 756f8537
---
title: Disallow Git URLs that include a username or hostname beginning with a non-alphanumeric
character
merge_request:
author:
...@@ -19,6 +19,8 @@ module Gitlab ...@@ -19,6 +19,8 @@ module Gitlab
return false if internal?(uri) return false if internal?(uri)
return true if blocked_port?(uri.port) return true if blocked_port?(uri.port)
return true if blocked_user_or_hostname?(uri.user)
return true if blocked_user_or_hostname?(uri.hostname)
server_ips = Resolv.getaddresses(uri.hostname) server_ips = Resolv.getaddresses(uri.hostname)
return true if (blocked_ips & server_ips).any? return true if (blocked_ips & server_ips).any?
...@@ -37,6 +39,12 @@ module Gitlab ...@@ -37,6 +39,12 @@ module Gitlab
port < 1024 && !VALID_PORTS.include?(port) port < 1024 && !VALID_PORTS.include?(port)
end end
def blocked_user_or_hostname?(value)
return false if value.blank?
value !~ /\A\p{Alnum}/
end
def internal?(uri) def internal?(uri)
internal_web?(uri) || internal_shell?(uri) internal_web?(uri) || internal_shell?(uri)
end end
......
...@@ -20,6 +20,34 @@ describe Gitlab::UrlBlocker, lib: true do ...@@ -20,6 +20,34 @@ describe Gitlab::UrlBlocker, lib: true do
expect(described_class.blocked_url?('https://gitlab.com:25/foo/foo.git')).to be true expect(described_class.blocked_url?('https://gitlab.com:25/foo/foo.git')).to be true
end 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')
# The leading character here is a Unicode "soft hyphen"
expect(described_class).to be_blocked_url('ssh://­oProxyCommand=whoami/a')
# Unicode alphanumerics are allowed
expect(described_class).not_to be_blocked_url('ssh://ğitlab.com/a')
end
end
it 'returns true for a non-alphanumeric username' do
stub_resolv
aggregate_failures do
expect(described_class).to be_blocked_url('ssh://-oProxyCommand=whoami@example.com/a')
# The leading character here is a Unicode "soft hyphen"
expect(described_class).to be_blocked_url('ssh://­oProxyCommand=whoami@example.com/a')
# Unicode alphanumerics are allowed
expect(described_class).not_to be_blocked_url('ssh://ğitlab@example.com/a')
end
end
it 'returns true for invalid URL' do it 'returns true for invalid URL' do
expect(described_class.blocked_url?('http://:8080')).to be true expect(described_class.blocked_url?('http://:8080')).to be true
end end
...@@ -28,4 +56,10 @@ describe Gitlab::UrlBlocker, lib: true do ...@@ -28,4 +56,10 @@ describe Gitlab::UrlBlocker, lib: true do
expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git')).to be false expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git')).to be false
end 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 end
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