Re: [PATCH] urlmatch: create fetch.credentialsInUrl config

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Stolee,

On Mon, 23 May 2022, Derrick Stolee via GitGitGadget wrote:

> diff --git a/urlmatch.c b/urlmatch.c
> index b615adc923a..6b91fb648a7 100644
> --- a/urlmatch.c
> +++ b/urlmatch.c
> +static void detected_credentials_in_url(const char *url, size_t scheme_len)
> +{
> +	char *value = NULL;
> +	const char *at_ptr;
> +	const char *colon_ptr;
> +	struct strbuf anonymized = STRBUF_INIT;
> +
> +	/* "ignore" is the default behavior. */
> +	if (git_config_get_string("fetch.credentialsinurl", &value) ||
> +	    !strcasecmp("ignore", value))
> +		goto cleanup;
> +
> +	at_ptr = strchr(url, '@');
> +	colon_ptr = strchr(url + scheme_len + 3, ':');

How certain are we that `url + scheme_len + 3` is still inside the
NUL-separated `url`?

> +
> +	if (!colon_ptr)
> +		BUG("failed to find colon in url '%s' with scheme_len %"PRIuMAX,
> +		    url, (uintmax_t) scheme_len);

Wouldn't this mean that `https://github.com/git/git` with a `scheme_len`
of 5 would hit that `BUG()` code path?

Thanks,
Dscho

> +
> +	/* Include everything including the colon. */
> +	colon_ptr++;
> +	strbuf_add(&anonymized, url, colon_ptr - url);
> +
> +	while (colon_ptr < at_ptr) {
> +		strbuf_addch(&anonymized, '*');
> +		colon_ptr++;
> +	}
> +
> +	strbuf_addstr(&anonymized, at_ptr);
> +
> +	if (!strcasecmp("warn", value))
> +		warning(_("URL '%s' uses plaintext credentials"), anonymized.buf);
> +	if (!strcasecmp("die", value))
> +		die(_("URL '%s' uses plaintext credentials"), anonymized.buf);
> +
> +cleanup:
> +	free(value);
> +	strbuf_release(&anonymized);
> +}
> +
>  static char *url_normalize_1(const char *url, struct url_info *out_info, char allow_globs)
>  {
>  	/*