RE: [PATCH 2/2] http: prevent redirect from dropping credentials during reauth

[<rsbecker@nexbridge.com>]

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@devyard.org>
>---
> 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