Merge branch '56020-reduce-cyclomatic-complexity' into 'master'
Reduce cyclomatic complexity of UrlBlocker.validate! See merge request gitlab-org/gitlab-ce!30557
This commit is contained in:
commit
a296ae9950
|
@ -18,7 +18,6 @@ module Gitlab
|
|||
# enforce_sanitization - Raises error if URL includes any HTML/CSS/JS tags and argument is true.
|
||||
#
|
||||
# Returns an array with [<uri>, <original-hostname>].
|
||||
# rubocop:disable Metrics/CyclomaticComplexity
|
||||
# rubocop:disable Metrics/ParameterLists
|
||||
def validate!(
|
||||
url,
|
||||
|
@ -30,7 +29,6 @@ module Gitlab
|
|||
enforce_user: false,
|
||||
enforce_sanitization: false,
|
||||
dns_rebind_protection: true)
|
||||
# rubocop:enable Metrics/CyclomaticComplexity
|
||||
# rubocop:enable Metrics/ParameterLists
|
||||
|
||||
return [nil, nil] if url.nil?
|
||||
|
@ -38,36 +36,31 @@ module Gitlab
|
|||
# Param url can be a string, URI or Addressable::URI
|
||||
uri = parse_url(url)
|
||||
|
||||
validate_html_tags!(uri) if enforce_sanitization
|
||||
validate_uri(
|
||||
uri: uri,
|
||||
schemes: schemes,
|
||||
ports: ports,
|
||||
enforce_sanitization: enforce_sanitization,
|
||||
enforce_user: enforce_user,
|
||||
ascii_only: ascii_only
|
||||
)
|
||||
|
||||
hostname = uri.hostname
|
||||
port = get_port(uri)
|
||||
|
||||
unless internal?(uri)
|
||||
validate_scheme!(uri.scheme, schemes)
|
||||
validate_port!(port, ports) if ports.any?
|
||||
validate_user!(uri.user) if enforce_user
|
||||
validate_hostname!(hostname)
|
||||
validate_unicode_restriction!(uri) if ascii_only
|
||||
end
|
||||
address_info = get_address_info(hostname, port)
|
||||
return [uri, nil] unless address_info
|
||||
|
||||
begin
|
||||
addrs_info = Addrinfo.getaddrinfo(hostname, port, nil, :STREAM).map do |addr|
|
||||
addr.ipv6_v4mapped? ? addr.ipv6_to_ipv4 : addr
|
||||
end
|
||||
rescue SocketError
|
||||
return [uri, nil]
|
||||
end
|
||||
|
||||
protected_uri_with_hostname = enforce_uri_hostname(addrs_info, uri, hostname, dns_rebind_protection)
|
||||
protected_uri_with_hostname = enforce_uri_hostname(address_info, uri, hostname, dns_rebind_protection)
|
||||
|
||||
# Allow url from the GitLab instance itself but only for the configured hostname and ports
|
||||
return protected_uri_with_hostname if internal?(uri)
|
||||
|
||||
validate_localhost!(addrs_info) unless allow_localhost
|
||||
validate_loopback!(addrs_info) unless allow_localhost
|
||||
validate_local_network!(addrs_info) unless allow_local_network
|
||||
validate_link_local!(addrs_info) unless allow_local_network
|
||||
validate_local_request(
|
||||
address_info: address_info,
|
||||
allow_localhost: allow_localhost,
|
||||
allow_local_network: allow_local_network
|
||||
)
|
||||
|
||||
protected_uri_with_hostname
|
||||
end
|
||||
|
@ -101,11 +94,44 @@ module Gitlab
|
|||
[uri, hostname]
|
||||
end
|
||||
|
||||
def validate_uri(uri:, schemes:, ports:, enforce_sanitization:, enforce_user:, ascii_only:)
|
||||
validate_html_tags(uri) if enforce_sanitization
|
||||
|
||||
return if internal?(uri)
|
||||
|
||||
validate_scheme(uri.scheme, schemes)
|
||||
validate_port(get_port(uri), ports) if ports.any?
|
||||
validate_user(uri.user) if enforce_user
|
||||
validate_hostname(uri.hostname)
|
||||
validate_unicode_restriction(uri) if ascii_only
|
||||
end
|
||||
|
||||
def get_address_info(hostname, port)
|
||||
Addrinfo.getaddrinfo(hostname, port, nil, :STREAM).map do |addr|
|
||||
addr.ipv6_v4mapped? ? addr.ipv6_to_ipv4 : addr
|
||||
end
|
||||
rescue SocketError
|
||||
end
|
||||
|
||||
def validate_local_request(address_info:, allow_localhost:, allow_local_network:)
|
||||
return if allow_local_network && allow_localhost
|
||||
|
||||
unless allow_localhost
|
||||
validate_localhost(address_info)
|
||||
validate_loopback(address_info)
|
||||
end
|
||||
|
||||
unless allow_local_network
|
||||
validate_local_network(address_info)
|
||||
validate_link_local(address_info)
|
||||
end
|
||||
end
|
||||
|
||||
def get_port(uri)
|
||||
uri.port || uri.default_port
|
||||
end
|
||||
|
||||
def validate_html_tags!(uri)
|
||||
def validate_html_tags(uri)
|
||||
uri_str = uri.to_s
|
||||
sanitized_uri = ActionController::Base.helpers.sanitize(uri_str, tags: [])
|
||||
if sanitized_uri != uri_str
|
||||
|
@ -125,7 +151,7 @@ module Gitlab
|
|||
CGI.unescape(url.to_s) =~ /\n|\r/
|
||||
end
|
||||
|
||||
def validate_port!(port, ports)
|
||||
def validate_port(port, ports)
|
||||
return if port.blank?
|
||||
# Only ports under 1024 are restricted
|
||||
return if port >= 1024
|
||||
|
@ -134,20 +160,20 @@ module Gitlab
|
|||
raise BlockedUrlError, "Only allowed ports are #{ports.join(', ')}, and any over 1024"
|
||||
end
|
||||
|
||||
def validate_scheme!(scheme, schemes)
|
||||
def validate_scheme(scheme, schemes)
|
||||
if scheme.blank? || (schemes.any? && !schemes.include?(scheme))
|
||||
raise BlockedUrlError, "Only allowed schemes are #{schemes.join(', ')}"
|
||||
end
|
||||
end
|
||||
|
||||
def validate_user!(value)
|
||||
def validate_user(value)
|
||||
return if value.blank?
|
||||
return if value =~ /\A\p{Alnum}/
|
||||
|
||||
raise BlockedUrlError, "Username needs to start with an alphanumeric character"
|
||||
end
|
||||
|
||||
def validate_hostname!(value)
|
||||
def validate_hostname(value)
|
||||
return if value.blank?
|
||||
return if IPAddress.valid?(value)
|
||||
return if value =~ /\A\p{Alnum}/
|
||||
|
@ -155,13 +181,13 @@ module Gitlab
|
|||
raise BlockedUrlError, "Hostname or IP address invalid"
|
||||
end
|
||||
|
||||
def validate_unicode_restriction!(uri)
|
||||
def validate_unicode_restriction(uri)
|
||||
return if uri.to_s.ascii_only?
|
||||
|
||||
raise BlockedUrlError, "URI must be ascii only #{uri.to_s.dump}"
|
||||
end
|
||||
|
||||
def validate_localhost!(addrs_info)
|
||||
def validate_localhost(addrs_info)
|
||||
local_ips = ["::", "0.0.0.0"]
|
||||
local_ips.concat(Socket.ip_address_list.map(&:ip_address))
|
||||
|
||||
|
@ -170,19 +196,19 @@ module Gitlab
|
|||
raise BlockedUrlError, "Requests to localhost are not allowed"
|
||||
end
|
||||
|
||||
def validate_loopback!(addrs_info)
|
||||
def validate_loopback(addrs_info)
|
||||
return unless addrs_info.any? { |addr| addr.ipv4_loopback? || addr.ipv6_loopback? }
|
||||
|
||||
raise BlockedUrlError, "Requests to loopback addresses are not allowed"
|
||||
end
|
||||
|
||||
def validate_local_network!(addrs_info)
|
||||
def validate_local_network(addrs_info)
|
||||
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
|
||||
|
||||
def validate_link_local!(addrs_info)
|
||||
def validate_link_local(addrs_info)
|
||||
netmask = IPAddr.new('169.254.0.0/16')
|
||||
return unless addrs_info.any? { |addr| addr.ipv6_linklocal? || netmask.include?(addr.ip_address) }
|
||||
|
||||
|
|
|
@ -353,7 +353,7 @@ describe Gitlab::UrlBlocker do
|
|||
end
|
||||
end
|
||||
|
||||
describe '#validate_hostname!' do
|
||||
describe '#validate_hostname' do
|
||||
let(:ip_addresses) do
|
||||
[
|
||||
'2001:db8:1f70::999:de8:7648:6e8',
|
||||
|
@ -378,7 +378,7 @@ describe Gitlab::UrlBlocker do
|
|||
|
||||
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
|
||||
expect { described_class.send(:validate_hostname, ip) }.not_to raise_error
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue