On Sunday, February 4, 2024 1:54 PM, Quentin Bouget wrote:
>During a re-authentication (second attempt at authenticating with a remote,
e.g.
>after a failed GSSAPI attempt), git allows the remote to provide credential
overrides
>in the redirect URL and unconditionnaly drops the current HTTP credentials
in favors
>of those, even when there aren't any.
>
>This commit makes it so HTTP credentials are only overridden when the
redirect URL
>actually contains credentials itself.
>
>Signed-off-by: Quentin Bouget <ypsah@xxxxxxxxxxx>
>---
> http.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
>diff --git a/http.c b/http.c
>index ccea19ac47..caba9cac1e 100644
>--- a/http.c
>+++ b/http.c
>@@ -2160,7 +2160,25 @@ static int http_request_reauth(const char *url,
> if (options && options->effective_url && options->base_url) {
> if (update_url_from_redirect(options->base_url,
> url, options->effective_url)) {
>+ char *username = NULL, *password = NULL;
>+
>+ if (http_auth.username)
>+ username = xstrdup(http_auth.username);
>+ if (http_auth.password)
>+ password = xstrdup(http_auth.password);
>+
> credential_from_url(&http_auth,
options->base_url->buf);
>+
>+ if (http_auth.username)
>+ free(username);
>+ else if (username)
>+ http_auth.username = username;
>+
>+ if (http_auth.password)
>+ free(password);
>+ else if (password)
>+ http_auth.password = password;
>+
> url = options->effective_url->buf;
> }
> }
I am wondering whether this is a good idea. Having credentials in a redirect
seems like it might be a vector for going somewhere other than what you want
to do, with credentials you do not necessarily want. Others might no better
than I on this, but would potentially lead to a CVE? I would prefer to see
credentials in a redirect rejected rather than used.
--Randall
