Commit b2969216 authored by James Edwards-Jones's avatar James Edwards-Jones

Merge branch 'rs-alphanumeric-ssh-params' into 'security-9-4'

Ensure user and hostnames begin with an alnum character in UrlBlocker

See merge request !2138
parent 334915d5
Pipeline #10775514 passed with stages
in 112 minutes and 53 seconds
---
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
return false if internal?(uri)
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)
return true if (blocked_ips & server_ips).any?
......@@ -37,6 +39,12 @@ module Gitlab
port < 1024 && !VALID_PORTS.include?(port)
end
def blocked_user_or_hostname?(value)
return false if value.blank?
value !~ /\A\p{Alnum}/
end
def internal?(uri)
internal_web?(uri) || internal_shell?(uri)
end
......
......@@ -20,6 +20,34 @@ describe Gitlab::UrlBlocker do
expect(described_class.blocked_url?('https://gitlab.com:25/foo/foo.git')).to be true
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
expect(described_class.blocked_url?('http://:8080')).to be true
end
......@@ -28,4 +56,10 @@ describe Gitlab::UrlBlocker do
expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git')).to be false
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
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