Commit 50ff3626 authored by Stan Hu's avatar Stan Hu

Escape username and password in UrlSanitizer#full_url

If a user uses a password with certain characters (e.g. /, #, +, etc.)
UrlSanitizer#full_url will generate an invalid URL that cannot be
parsed properly by Addressable::URI. If used with UrlBlocker, this
will be flagged as an invalid URI.
parent 98eccfc4
---
title: Escape username and password in UrlSanitizer#full_url
merge_request: 20684
author:
type: fixed
...@@ -71,12 +71,10 @@ module Gitlab ...@@ -71,12 +71,10 @@ module Gitlab
def generate_full_url def generate_full_url
return @url unless valid_credentials? return @url unless valid_credentials?
@full_url = @url.dup @url.dup.tap do |generated|
generated.password = encode_percent(credentials[:password]) if credentials[:password].present?
@full_url.password = credentials[:password] if credentials[:password].present? generated.user = encode_percent(credentials[:user]) if credentials[:user].present?
@full_url.user = credentials[:user] if credentials[:user].present? end
@full_url
end end
def safe_url def safe_url
...@@ -89,5 +87,10 @@ module Gitlab ...@@ -89,5 +87,10 @@ module Gitlab
def valid_credentials? def valid_credentials?
credentials && credentials.is_a?(Hash) && credentials.any? credentials && credentials.is_a?(Hash) && credentials.any?
end end
def encode_percent(string)
# CGI.escape converts spaces to +, but this doesn't work for git clone
CGI.escape(string).gsub('+', '%20')
end
end end
end end
...@@ -145,6 +145,10 @@ describe Gitlab::UrlSanitizer do ...@@ -145,6 +145,10 @@ describe Gitlab::UrlSanitizer do
'http://foo:@example.com' | 'http://foo@example.com' 'http://foo:@example.com' | 'http://foo@example.com'
'http://:bar@example.com' | :same 'http://:bar@example.com' | :same
'http://foo:bar@example.com' | :same 'http://foo:bar@example.com' | :same
'http://foo:g p@example.com' | 'http://foo:g%20p@example.com'
'http://foo:s/h@example.com' | 'http://foo:s%2Fh@example.com'
'http://t u:a#b@example.com' | 'http://t%20u:a%23b@example.com'
'http://t+u:a#b@example.com' | 'http://t%2Bu:a%23b@example.com'
end end
with_them do with_them do
...@@ -160,7 +164,7 @@ describe Gitlab::UrlSanitizer do ...@@ -160,7 +164,7 @@ describe Gitlab::UrlSanitizer do
url_sanitizer = described_class.new("https://foo:b?r@github.com/me/project.git") url_sanitizer = described_class.new("https://foo:b?r@github.com/me/project.git")
expect(url_sanitizer.sanitized_url).to eq("https://github.com/me/project.git") expect(url_sanitizer.sanitized_url).to eq("https://github.com/me/project.git")
expect(url_sanitizer.full_url).to eq("https://foo:b?r@github.com/me/project.git") expect(url_sanitizer.full_url).to eq("https://foo:b%3Fr@github.com/me/project.git")
end end
end end
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