Merge branch 'security-scp-url-sanitizer-17-1' into '17-1-stable-ee'

Fix credentials disclosure in mirroring failure

See merge request https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/4448



Merged-by: GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com>
Approved-by: Vasilii Iakliushin <viakliushin@gitlab.com>
Co-authored-by: Olaoluwa Oluro <olaoluro@gitlab.com>

lib/gitlab/url_sanitizer.rb

@@ -8,6 +8,12 @@ class UrlSanitizer
     ALLOWED_WEB_SCHEMES = %w[http https].freeze
     SCHEMIFIED_SCHEME = 'glschemelessuri'
     SCHEMIFY_PLACEHOLDER = "#{SCHEMIFIED_SCHEME}://".freeze
+    # SCP style URLs have a format of [userinfo]@[host]:[path] with them not containing
+    # port arguments as that is passed along with a -P argument
+    SCP_REGEX = %r{
+      #{URI::REGEXP::PATTERN::USERINFO}@#{URI::REGEXP::PATTERN::HOST}:
+      (?!\b\d+\b) # use word boundaries to ensure no standalone digits after the colon
+    }x
     # URI::DEFAULT_PARSER.make_regexp will only match URLs with schemes or
     # relative URLs. This section will match schemeless URIs with userinfo
     # e.g. user:pass@gitlab.com but will not match scp-style URIs e.g.
@@ -20,9 +26,9 @@ class UrlSanitizer
     (?:
        #{URI::DEFAULT_PARSER.make_regexp(ALLOWED_SCHEMES)}
      |
+       (?# negative lookahead before the schemeless matcher ensures this isn't an SCP-style URL)
+       (?!#{SCP_REGEX})
        (?:(?:(?!@)[%#{URI::REGEXP::PATTERN::UNRESERVED}#{URI::REGEXP::PATTERN::RESERVED}])+(?:@))
-       (?# negative lookahead ensures this isn't an SCP-style URL: [host]:[rel_path|abs_path] server:path/to/file)
-       (?!#{URI::REGEXP::PATTERN::HOST}:(?:#{URI::REGEXP::PATTERN::REL_PATH}|#{URI::REGEXP::PATTERN::ABS_PATH}))
        #{URI::REGEXP::PATTERN::HOSTPORT}
     )
     }x

spec/lib/gitlab/url_sanitizer_spec.rb

@@ -33,6 +33,16 @@ def sanitize_url(url)
       urls << ['user:@server:project.git', 'user:@server:project.git']
       urls << [':pass@server:project.git', ':pass@server:project.git']
       urls << ['user:pass@server:project.git', 'user:pass@server:project.git']
+      urls << ['user:pass@server:123project.git', 'user:pass@server:123project.git']
+      urls << ['user:pass@server:1project3.git', 'user:pass@server:1project3.git']
+      urls << ['user:pass@server:project123.git', 'user:pass@server:project123.git']
+      urls << ['root@host:/root/ids/rules.tar.gz', 'root@host:/root/ids/rules.tar.gz']
+
+      # actual URLs that look like SCP-styled URLS
+      urls << ['username:password@test.com', '*****:*****@test.com']
+      urls << ['username:password@test.com:1234', '*****:*****@test.com:1234']
+      urls << ['username:password@test.com:1234/org/project', '*****:*****@test.com:1234/org/project']
+      urls << ['username:password@test.com:1234/org/project.git', '*****:*****@test.com:1234/org/project.git']
       # return an empty string for invalid URLs
       urls << ['ssh://', '']