From 50ff36265016728ab9372bff6b16b49e2d2364d6 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 17 Jul 2018 22:50:08 -0700 Subject: [PATCH] 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. --- changelogs/unreleased/sh-normalize-urls.yml | 5 +++++ lib/gitlab/url_sanitizer.rb | 15 +++++++++------ spec/lib/gitlab/url_sanitizer_spec.rb | 6 +++++- 3 files changed, 19 insertions(+), 7 deletions(-) create mode 100644 changelogs/unreleased/sh-normalize-urls.yml diff --git a/changelogs/unreleased/sh-normalize-urls.yml b/changelogs/unreleased/sh-normalize-urls.yml new file mode 100644 index 00000000000..b0d1120e10b --- /dev/null +++ b/changelogs/unreleased/sh-normalize-urls.yml @@ -0,0 +1,5 @@ +--- +title: Escape username and password in UrlSanitizer#full_url +merge_request: 20684 +author: +type: fixed diff --git a/lib/gitlab/url_sanitizer.rb b/lib/gitlab/url_sanitizer.rb index de8b6ec69ce..308a95d2f09 100644 --- a/lib/gitlab/url_sanitizer.rb +++ b/lib/gitlab/url_sanitizer.rb @@ -71,12 +71,10 @@ module Gitlab def generate_full_url return @url unless valid_credentials? - @full_url = @url.dup - - @full_url.password = credentials[:password] if credentials[:password].present? - @full_url.user = credentials[:user] if credentials[:user].present? - - @full_url + @url.dup.tap do |generated| + generated.password = encode_percent(credentials[:password]) if credentials[:password].present? + generated.user = encode_percent(credentials[:user]) if credentials[:user].present? + end end def safe_url @@ -89,5 +87,10 @@ module Gitlab def valid_credentials? credentials && credentials.is_a?(Hash) && credentials.any? 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 diff --git a/spec/lib/gitlab/url_sanitizer_spec.rb b/spec/lib/gitlab/url_sanitizer_spec.rb index 758a9bc5a2b..b41a81a8167 100644 --- a/spec/lib/gitlab/url_sanitizer_spec.rb +++ b/spec/lib/gitlab/url_sanitizer_spec.rb @@ -145,6 +145,10 @@ describe Gitlab::UrlSanitizer do 'http://foo:@example.com' | 'http://foo@example.com' 'http://: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 with_them do @@ -160,7 +164,7 @@ describe Gitlab::UrlSanitizer do 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.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 -- GitLab