On Tue, Jan 09, 2024 at 05:53:37PM +0000, Victoria Dye via GitGitGadget wrote: > From: Victoria Dye <vdye@xxxxxxxxxx> > > Update the validation of "curl URL" submodule URLs (i.e. those that specify > an "http[s]" or "ftp[s]" protocol) in 'check_submodule_url()' to catch more > invalid URLs. The existing validation using 'credential_from_url_gently()' > parses certain URLs incorrectly, leading to invalid submodule URLs passing > 'git fsck' checks. Conversely, 'url_normalize()' - used to validate remote > URLs in 'remote_get()' - correctly identifies the invalid URLs missed by > 'credential_from_url_gently()'. Okay, so we retain the wrong behavior of `credential_from_url_gently()`, right? I wonder whether this can be abused in any way, doubly so because the function gets invoked with untrusted input from the remote server when we handle redirects in `http_request_reauth()`. But the redirect URL we end up passing to `credential_from_url_gently()` would have to contain a non-numeric port, and curl seemingly does not know to handle those either. Other callsites include fsck (which you're fixing) and the credential store (which is entirely user-controlled). It would be great regardless to fix the underlying bug in `credential_from_url_gently()` eventually though. But I do not think that this has to be part this patch series here, which is a strict improvement. Thanks! Patrick
Attachment:
signature.asc
Description: PGP signature
