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

> 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.