diff options
| -rw-r--r-- | lib/gitlab/url_blocker.rb | 80 | ||||
| -rw-r--r-- | spec/models/project_spec.rb | 4 | ||||
| -rw-r--r-- | spec/services/projects/import_service_spec.rb | 4 |
3 files changed, 51 insertions, 37 deletions
diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb index af6c389b4a2..f2c97791b9d 100644 --- a/lib/gitlab/url_blocker.rb +++ b/lib/gitlab/url_blocker.rb @@ -10,28 +10,27 @@ module Gitlab begin uri = Addressable::URI.parse(url) - # Allow imports from the GitLab instance itself but only from the configured ports - return true if internal?(uri) - - raise BlockedUrlError, "Port is blocked" if blocked_port?(uri.port, valid_ports) - raise BlockedUrlError, "User is blocked" if blocked_user_or_hostname?(uri.user) - raise BlockedUrlError, "Hostname is blocked" if blocked_user_or_hostname?(uri.hostname) + rescue Addressable::URI::InvalidURIError + raise BlockedUrlError, "URI is invalid" + end - addrs_info = Addrinfo.getaddrinfo(uri.hostname, 80, nil, :STREAM) + # Allow imports from the GitLab instance itself but only from the configured ports + return true if internal?(uri) - if !allow_localhost && localhost?(addrs_info) - raise BlockedUrlError, "Requests to localhost are blocked" - end + port = uri.port || uri.default_port + validate_port!(port, valid_ports) if valid_ports.any? + validate_user!(uri.user) + validate_hostname!(uri.hostname) - if !allow_private_networks && private_network?(addrs_info) - raise BlockedUrlError, "Requests to the private local network are blocked" - end - rescue Addressable::URI::InvalidURIError - raise BlockedUrlError, "URI is invalid" + begin + addrs_info = Addrinfo.getaddrinfo(uri.hostname, port, nil, :STREAM) rescue SocketError - return + return true end + validate_localhost!(addrs_info) unless allow_localhost + validate_local_network!(addrs_info) unless allow_private_networks + true end @@ -45,16 +44,42 @@ module Gitlab private - def blocked_port?(port, valid_ports) - return false if port.blank? || valid_ports.blank? + def validate_port!(port, valid_ports) + return if port.blank? + # Only ports under 1024 are restricted + return if port >= 1024 + return if valid_ports.include?(port) + + raise BlockedUrlError, "Only allowed ports are #{valid_ports.join(', ')}, and any over 1024" + end + + def validate_user!(value) + return if value.blank? + return if value =~ /\A\p{Alnum}/ - port < 1024 && !valid_ports.include?(port) + raise BlockedUrlError, "Username needs to start with an alphanumeric character" end - def blocked_user_or_hostname?(value) - return false if value.blank? + def validate_hostname!(value) + return if value.blank? + return if value =~ /\A\p{Alnum}/ - value !~ /\A\p{Alnum}/ + raise BlockedUrlError, "Hostname needs to start with an alphanumeric character" + end + + def validate_localhost!(addrs_info) + local_ips = ["127.0.0.1", "::1", "0.0.0.0"] + local_ips.concat(Socket.ip_address_list.map(&:ip_address)) + + return if (local_ips & addrs_info.map(&:ip_address)).empty? + + raise BlockedUrlError, "Requests to localhost are not allowed" + end + + def validate_local_network!(addrs_info) + return unless addrs_info.any? { |addr| addr.ipv4_private? || addr.ipv6_sitelocal? } + + raise BlockedUrlError, "Requests to the local network are not allowed" end def internal?(uri) @@ -71,17 +96,6 @@ module Gitlab (uri.port.blank? || uri.port == config.gitlab_shell.ssh_port) end - def localhost?(addrs_info) - blocked_ips = ["127.0.0.1", "::1", "0.0.0.0"] - blocked_ips.concat(Socket.ip_address_list.map(&:ip_address)) - - (blocked_ips & addrs_info.map(&:ip_address)).any? - end - - def private_network?(addrs_info) - addrs_info.any? { |addr| addr.ipv4_private? || addr.ipv6_sitelocal? } - end - def config Gitlab.config end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 96adf64bcec..fef868ac0f2 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -224,14 +224,14 @@ describe Project do project2 = build(:project, import_url: 'http://localhost:9000/t.git') expect(project2).to be_invalid - expect(project2.errors[:import_url]).to include('imports are not allowed from that URL') + expect(project2.errors[:import_url].first).to include('Requests to localhost are not allowed') end it "does not allow blocked import_url port" do project2 = build(:project, import_url: 'http://github.com:25/t.git') expect(project2).to be_invalid - expect(project2.errors[:import_url]).to include('imports are not allowed from that URL') + expect(project2.errors[:import_url].first).to include('Only allowed ports are 22, 80, 443') end describe 'project pending deletion' do diff --git a/spec/services/projects/import_service_spec.rb b/spec/services/projects/import_service_spec.rb index bf7facaec99..30c89ebd821 100644 --- a/spec/services/projects/import_service_spec.rb +++ b/spec/services/projects/import_service_spec.rb @@ -156,7 +156,7 @@ describe Projects::ImportService do result = described_class.new(project, user).execute expect(result[:status]).to eq :error - expect(result[:message]).to end_with 'Blocked import URL.' + expect(result[:message]).to include('Requests to localhost are not allowed') end it 'fails with port 25' do @@ -165,7 +165,7 @@ describe Projects::ImportService do result = described_class.new(project, user).execute expect(result[:status]).to eq :error - expect(result[:message]).to end_with 'Blocked import URL.' + expect(result[:message]).to include('Only allowed ports are 22, 80, 443') end end |