Skip to content

Validate the scheme of project import URLs

Dominic Couture requested to merge dcouture-import-scheme-validation into master

What does this MR do and why?

Project::ImportService requires that the import URL's port is either 80 or 443 but there's no scheme validation. Strangely enough, this validation passes fine with the file:// scheme.

[2] pry(main)> Gitlab::UrlBlocker.validate!('file://example.com', ports: Project::VALID_IMPORT_PORTS)
=> [#<Addressable::URI:0x3d7fc URI:file://93.184.216.34>, "example.com"]

This is unlikely to lead to anything too bad given that a valid host is still required and we did not find this to be exploitable, but to be safe I'm adding scheme validation in this MR.

Given that we validate that the port is either 80 or 443 I assumed that we're looking for http or https schemes here and don't expect anything else like ssh or git but I would like a ~"group::import" SME to validate.

Related to, but does not close #372317 (closed)

Screenshots or screen recordings

N/A

How to set up and validate locally

Defense in depth validation that I could only trigger with the unit test, though there might be a roundabout way to get a non-http URL there I did not find it.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Dominic Couture

Merge request reports