From 72c00594070dfd1a778c2e03ff400b478e6c3774 Mon Sep 17 00:00:00 2001 From: James Edwards-Jones Date: Wed, 5 Dec 2018 20:14:09 +0000 Subject: Allow URLs to be validated as ascii_only Restricts unicode characters and IDNA deviations which could be used in a phishing attack --- app/validators/url_validator.rb | 1 + lib/gitlab/url_blocker.rb | 9 ++++++++- spec/lib/gitlab/url_blocker_spec.rb | 21 +++++++++++++++++++++ spec/validators/url_validator_spec.rb | 29 +++++++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 1 deletion(-) diff --git a/app/validators/url_validator.rb b/app/validators/url_validator.rb index 216acf79cbd..5feb0b0f05b 100644 --- a/app/validators/url_validator.rb +++ b/app/validators/url_validator.rb @@ -69,6 +69,7 @@ class UrlValidator < ActiveModel::EachValidator ports: [], allow_localhost: true, allow_local_network: true, + ascii_only: false, enforce_user: false } end diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb index b8040f73cee..44c71f8431d 100644 --- a/lib/gitlab/url_blocker.rb +++ b/lib/gitlab/url_blocker.rb @@ -8,7 +8,7 @@ module Gitlab BlockedUrlError = Class.new(StandardError) class << self - def validate!(url, allow_localhost: false, allow_local_network: true, enforce_user: false, ports: [], protocols: []) + def validate!(url, ports: [], protocols: [], allow_localhost: false, allow_local_network: true, ascii_only: false, enforce_user: false) return true if url.nil? # Param url can be a string, URI or Addressable::URI @@ -22,6 +22,7 @@ module Gitlab validate_port!(port, ports) if ports.any? validate_user!(uri.user) if enforce_user validate_hostname!(uri.hostname) + validate_unicode_restriction!(uri) if ascii_only begin addrs_info = Addrinfo.getaddrinfo(uri.hostname, port, nil, :STREAM).map do |addr| @@ -91,6 +92,12 @@ module Gitlab raise BlockedUrlError, "Hostname or IP address invalid" end + 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) local_ips = ["::", "0.0.0.0"] local_ips.concat(Socket.ip_address_list.map(&:ip_address)) diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb index 39e0a17a307..62970bd8cb6 100644 --- a/spec/lib/gitlab/url_blocker_spec.rb +++ b/spec/lib/gitlab/url_blocker_spec.rb @@ -249,6 +249,27 @@ describe Gitlab::UrlBlocker do end end end + + context 'when ascii_only is true' do + it 'returns true for unicode domain' do + expect(described_class.blocked_url?('https://𝕘itⅼαƄ.com/foo/foo.bar', ascii_only: true)).to be true + end + + it 'returns true for unicode tld' do + expect(described_class.blocked_url?('https://gitlab.ᴄοm/foo/foo.bar', ascii_only: true)).to be true + end + + it 'returns true for unicode path' do + expect(described_class.blocked_url?('https://gitlab.com/𝒇οο/𝒇οο.Ƅαꮁ', ascii_only: true)).to be true + end + + it 'returns true for IDNA deviations' do + expect(described_class.blocked_url?('https://mißile.com/foo/foo.bar', ascii_only: true)).to be true + expect(described_class.blocked_url?('https://miςςile.com/foo/foo.bar', ascii_only: true)).to be true + expect(described_class.blocked_url?('https://git‍lab.com/foo/foo.bar', ascii_only: true)).to be true + expect(described_class.blocked_url?('https://git‌lab.com/foo/foo.bar', ascii_only: true)).to be true + end + end end describe '#validate_hostname!' do diff --git a/spec/validators/url_validator_spec.rb b/spec/validators/url_validator_spec.rb index 082d09d3f16..f3f3386382f 100644 --- a/spec/validators/url_validator_spec.rb +++ b/spec/validators/url_validator_spec.rb @@ -143,4 +143,33 @@ describe UrlValidator do end end end + + context 'when ascii_only is' do + let(:url) { 'https://𝕘itⅼαƄ.com/foo/foo.bar'} + let(:validator) { described_class.new(attributes: [:link_url], ascii_only: ascii_only) } + + context 'true' do + let(:ascii_only) { true } + + it 'prevents unicode characters' do + badge.link_url = url + + subject + + expect(badge.errors.empty?).to be false + end + end + + context 'false (default)' do + let(:ascii_only) { false } + + it 'does not prevent unicode characters' do + badge.link_url = url + + subject + + expect(badge.errors.empty?).to be true + end + end + end end -- cgit v1.2.3