Skip to content

Commit

Permalink
credential: optionally allow partial URLs in credential_from_url_gent…
Browse files Browse the repository at this point in the history
…ly()

Prior to the fixes for CVE-2020-11008, we were _very_ lenient in what we
required from a URL in order to parse it into a `struct credential`.
That led to serious vulnerabilities.

There was one call site, though, that really needed that leniency: when
parsing config settings a la `credential.dev.azure.com.useHTTPPath`.
Settings like this might be desired when users want to use, say, a given
user name on a given host, regardless of the protocol to be used.

In preparation for fixing that bug, let's refactor the code to
optionally allow for partial URLs. For the moment, this functionality is
only exposed via the now-renamed function `credential_from_url_1()`, but
it is not used. The intention is to make it easier to verify that this
commit does not change the existing behavior unless explicitly allowing
for partial URLs.

Please note that this patch does more than just reinstating a way to
imitate the behavior before those CVE-2020-11008 fixes: Before that, we
would simply ignore URLs without a protocol. In other words,
misleadingly, the following setting would be applied to _all_ URLs:

	[credential "example.com"]
		username = that-me

The obvious intention is to match the host name only. With this patch,
we allow precisely that: when parsing the URL with non-zero
`allow_partial_url`, we do not simply return success if there was no
protocol, but we simply leave the protocol unset and continue parsing
the URL.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Reviewed-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
dscho authored and gitster committed Apr 24, 2020
1 parent 21920cb commit 6828e59
Showing 1 changed file with 36 additions and 6 deletions.
42 changes: 36 additions & 6 deletions credential.c
Expand Up @@ -343,8 +343,31 @@ static int check_url_component(const char *url, int quiet,
return -1;
}

int credential_from_url_gently(struct credential *c, const char *url,
int quiet)
/*
* Potentially-partial URLs can, but do not have to, contain
*
* - a protocol (or scheme) of the form "<protocol>://"
*
* - a host name (the part after the protocol and before the first slash after
* that, if any)
*
* - a user name and potentially a password (as "<user>[:<password>]@" part of
* the host name)
*
* - a path (the part after the host name, if any, starting with the slash)
*
* Missing parts will be left unset in `struct credential`. Thus, `https://`
* will have only the `protocol` set, `example.com` only the host name, and
* `/git` only the path.
*
* Note that an empty host name in an otherwise fully-qualified URL (e.g.
* `cert:///path/to/cert.pem`) will be treated as unset if we expect the URL to
* be potentially partial, and only then (otherwise, the empty string is used).
*
* The credential_from_url() function does not allow partial URLs.
*/
static int credential_from_url_1(struct credential *c, const char *url,
int allow_partial_url, int quiet)
{
const char *at, *colon, *cp, *slash, *host, *proto_end;

Expand All @@ -357,12 +380,12 @@ int credential_from_url_gently(struct credential *c, const char *url,
* (3) proto://<user>:<pass>@<host>/...
*/
proto_end = strstr(url, "://");
if (!proto_end || proto_end == url) {
if (!allow_partial_url && (!proto_end || proto_end == url)) {
if (!quiet)
warning(_("url has no scheme: %s"), url);
return -1;
}
cp = proto_end + 3;
cp = proto_end ? proto_end + 3 : url;
at = strchr(cp, '@');
colon = strchr(cp, ':');
slash = strchrnul(cp, '/');
Expand All @@ -382,8 +405,10 @@ int credential_from_url_gently(struct credential *c, const char *url,
host = at + 1;
}

c->protocol = xmemdupz(url, proto_end - url);
c->host = url_decode_mem(host, slash - host);
if (proto_end && proto_end - url > 0)
c->protocol = xmemdupz(url, proto_end - url);
if (!allow_partial_url || slash - host > 0)
c->host = url_decode_mem(host, slash - host);
/* Trim leading and trailing slashes from path */
while (*slash == '/')
slash++;
Expand All @@ -405,6 +430,11 @@ int credential_from_url_gently(struct credential *c, const char *url,
return 0;
}

int credential_from_url_gently(struct credential *c, const char *url, int quiet)
{
return credential_from_url_1(c, url, 0, quiet);
}

void credential_from_url(struct credential *c, const char *url)
{
if (credential_from_url_gently(c, url, 0) < 0)
Expand Down

0 comments on commit 6828e59

Please sign in to comment.