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)
> {
> /*
