Allow URLs to be validated as ascii_only
Restricts unicode characters and IDNA deviations which could be used in a phishing attack
This commit is contained in:
		
							parent
							
								
									8cd5004b35
								
							
						
					
					
						commit
						72c0059407
					
				|  | @ -69,6 +69,7 @@ class UrlValidator < ActiveModel::EachValidator | |||
|       ports: [], | ||||
|       allow_localhost: true, | ||||
|       allow_local_network: true, | ||||
|       ascii_only: false, | ||||
|       enforce_user: false | ||||
|     } | ||||
|   end | ||||
|  |  | |||
|  | @ -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)) | ||||
|  |  | |||
|  | @ -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://gitlab.com/foo/foo.bar', ascii_only: true)).to be true | ||||
|         expect(described_class.blocked_url?('https://gitlab.com/foo/foo.bar', ascii_only: true)).to be true | ||||
|       end | ||||
|     end | ||||
|   end | ||||
| 
 | ||||
|   describe '#validate_hostname!' do | ||||
|  |  | |||
|  | @ -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 | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue