194 lines
7.6 KiB
Diff
194 lines
7.6 KiB
Diff
From b01b9b81d36759cdcd07305e78765199e1bc2060 Mon Sep 17 00:00:00 2001
|
|
From: Johannes Schindelin <johannes.schindelin@gmx.de>
|
|
Date: Mon, 4 Nov 2024 14:48:22 +0100
|
|
Subject: [PATCH] credential: disallow Carriage Returns in the protocol by
|
|
default
|
|
|
|
While Git has documented that the credential protocol is line-based,
|
|
with newlines as terminators, the exact shape of a newline has not been
|
|
documented.
|
|
|
|
From Git's perspective, which is firmly rooted in the Linux ecosystem,
|
|
it is clear that "a newline" means a Line Feed character.
|
|
|
|
However, even Git's credential protocol respects Windows line endings
|
|
(a Carriage Return character followed by a Line Feed character, "CR/LF")
|
|
by virtue of using `strbuf_getline()`.
|
|
|
|
There is a third category of line endings that has been used originally
|
|
by MacOS, and that is respected by the default line readers of .NET and
|
|
node.js: bare Carriage Returns.
|
|
|
|
Git cannot handle those, and what is worse: Git's remedy against
|
|
CVE-2020-5260 does not catch when credential helpers are used that
|
|
interpret bare Carriage Returns as newlines.
|
|
|
|
Git Credential Manager addressed this as CVE-2024-50338, but other
|
|
credential helpers may still be vulnerable. So let's not only disallow
|
|
Line Feed characters as part of the values in the credential protocol,
|
|
but also disallow Carriage Return characters.
|
|
|
|
In the unlikely event that a credential helper relies on Carriage
|
|
Returns in the protocol, introduce an escape hatch via the
|
|
`credential.protectProtocol` config setting.
|
|
|
|
This addresses CVE-2024-52006.
|
|
|
|
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
|
---
|
|
Documentation/config/credential.txt | 5 +++++
|
|
credential.c | 21 ++++++++++++++-------
|
|
credential.h | 4 +++-
|
|
t/t0300-credentials.sh | 16 ++++++++++++++++
|
|
4 files changed, 38 insertions(+), 8 deletions(-)
|
|
|
|
Index: b/Documentation/config/credential.txt
|
|
===================================================================
|
|
--- a/Documentation/config/credential.txt
|
|
+++ b/Documentation/config/credential.txt
|
|
@@ -20,6 +20,11 @@ credential.sanitizePrompt::
|
|
will be URL-encoded by default). Configure this setting to `false` to
|
|
override that behavior.
|
|
|
|
+credential.protectProtocol::
|
|
+ By default, Carriage Return characters are not allowed in the protocol
|
|
+ that is used when Git talks to a credential helper. This setting allows
|
|
+ users to override this default.
|
|
+
|
|
credential.username::
|
|
If no username is set for a network authentication, use this username
|
|
by default. See credential.<context>.* below, and
|
|
Index: b/credential.c
|
|
===================================================================
|
|
--- a/credential.c
|
|
+++ b/credential.c
|
|
@@ -127,6 +127,8 @@ static int credential_config_callback(co
|
|
c->use_http_path = git_config_bool(var, value);
|
|
else if (!strcmp(key, "sanitizeprompt"))
|
|
c->sanitize_prompt = git_config_bool(var, value);
|
|
+ else if (!strcmp(key, "protectprotocol"))
|
|
+ c->protect_protocol = git_config_bool(var, value);
|
|
|
|
return 0;
|
|
}
|
|
@@ -361,7 +363,8 @@ int credential_read(struct credential *c
|
|
return 0;
|
|
}
|
|
|
|
-static void credential_write_item(FILE *fp, const char *key, const char *value,
|
|
+static void credential_write_item(const struct credential *c,
|
|
+ FILE *fp, const char *key, const char *value,
|
|
int required)
|
|
{
|
|
if (!value && required)
|
|
@@ -370,6 +373,10 @@ static void credential_write_item(FILE *
|
|
return;
|
|
if (strchr(value, '\n'))
|
|
die("credential value for %s contains newline", key);
|
|
+ if (c->protect_protocol && strchr(value, '\r'))
|
|
+ die("credential value for %s contains carriage return\n"
|
|
+ "If this is intended, set `credential.protectProtocol=false`",
|
|
+ key);
|
|
fprintf(fp, "%s=%s\n", key, value);
|
|
}
|
|
|
|
@@ -377,34 +384,34 @@ void credential_write(const struct crede
|
|
enum credential_op_type op_type)
|
|
{
|
|
if (credential_has_capability(&c->capa_authtype, op_type))
|
|
- credential_write_item(fp, "capability[]", "authtype", 0);
|
|
+ credential_write_item(c, fp, "capability[]", "authtype", 0);
|
|
if (credential_has_capability(&c->capa_state, op_type))
|
|
- credential_write_item(fp, "capability[]", "state", 0);
|
|
+ credential_write_item(c, fp, "capability[]", "state", 0);
|
|
|
|
if (credential_has_capability(&c->capa_authtype, op_type)) {
|
|
- credential_write_item(fp, "authtype", c->authtype, 0);
|
|
- credential_write_item(fp, "credential", c->credential, 0);
|
|
+ credential_write_item(c, fp, "authtype", c->authtype, 0);
|
|
+ credential_write_item(c, fp, "credential", c->credential, 0);
|
|
if (c->ephemeral)
|
|
- credential_write_item(fp, "ephemeral", "1", 0);
|
|
+ credential_write_item(c, fp, "ephemeral", "1", 0);
|
|
}
|
|
- credential_write_item(fp, "protocol", c->protocol, 1);
|
|
- credential_write_item(fp, "host", c->host, 1);
|
|
- credential_write_item(fp, "path", c->path, 0);
|
|
- credential_write_item(fp, "username", c->username, 0);
|
|
- credential_write_item(fp, "password", c->password, 0);
|
|
- credential_write_item(fp, "oauth_refresh_token", c->oauth_refresh_token, 0);
|
|
+ credential_write_item(c, fp, "protocol", c->protocol, 1);
|
|
+ credential_write_item(c, fp, "host", c->host, 1);
|
|
+ credential_write_item(c, fp, "path", c->path, 0);
|
|
+ credential_write_item(c, fp, "username", c->username, 0);
|
|
+ credential_write_item(c, fp, "password", c->password, 0);
|
|
+ credential_write_item(c, fp, "oauth_refresh_token", c->oauth_refresh_token, 0);
|
|
if (c->password_expiry_utc != TIME_MAX) {
|
|
char *s = xstrfmt("%"PRItime, c->password_expiry_utc);
|
|
- credential_write_item(fp, "password_expiry_utc", s, 0);
|
|
+ credential_write_item(c, fp, "password_expiry_utc", s, 0);
|
|
free(s);
|
|
}
|
|
for (size_t i = 0; i < c->wwwauth_headers.nr; i++)
|
|
- credential_write_item(fp, "wwwauth[]", c->wwwauth_headers.v[i], 0);
|
|
+ credential_write_item(c, fp, "wwwauth[]", c->wwwauth_headers.v[i], 0);
|
|
if (credential_has_capability(&c->capa_state, op_type)) {
|
|
if (c->multistage)
|
|
- credential_write_item(fp, "continue", "1", 0);
|
|
+ credential_write_item(c, fp, "continue", "1", 0);
|
|
for (size_t i = 0; i < c->state_headers_to_send.nr; i++)
|
|
- credential_write_item(fp, "state[]", c->state_headers_to_send.v[i], 0);
|
|
+ credential_write_item(c, fp, "state[]", c->state_headers_to_send.v[i], 0);
|
|
}
|
|
}
|
|
|
|
Index: b/credential.h
|
|
===================================================================
|
|
--- a/credential.h
|
|
+++ b/credential.h
|
|
@@ -169,7 +169,8 @@ struct credential {
|
|
quit:1,
|
|
use_http_path:1,
|
|
username_from_proto:1,
|
|
- sanitize_prompt:1;
|
|
+ sanitize_prompt:1,
|
|
+ protect_protocol:1;
|
|
|
|
struct credential_capability capa_authtype;
|
|
struct credential_capability capa_state;
|
|
@@ -197,6 +198,7 @@ struct credential {
|
|
.state_headers = STRVEC_INIT, \
|
|
.state_headers_to_send = STRVEC_INIT, \
|
|
.sanitize_prompt = 1, \
|
|
+ .protect_protocol = 1, \
|
|
}
|
|
|
|
/* Initialize a credential structure, setting all fields to empty. */
|
|
Index: b/t/t0300-credentials.sh
|
|
===================================================================
|
|
--- a/t/t0300-credentials.sh
|
|
+++ b/t/t0300-credentials.sh
|
|
@@ -903,6 +903,22 @@ test_expect_success 'url parser rejects
|
|
test_cmp expect stderr
|
|
'
|
|
|
|
+test_expect_success 'url parser rejects embedded carriage returns' '
|
|
+ test_config credential.helper "!true" &&
|
|
+ test_must_fail git credential fill 2>stderr <<-\EOF &&
|
|
+ url=https://example%0d.com/
|
|
+ EOF
|
|
+ cat >expect <<-\EOF &&
|
|
+ fatal: credential value for host contains carriage return
|
|
+ If this is intended, set `credential.protectProtocol=false`
|
|
+ EOF
|
|
+ test_cmp expect stderr &&
|
|
+ GIT_ASKPASS=true \
|
|
+ git -c credential.protectProtocol=false credential fill <<-\EOF
|
|
+ url=https://example%0d.com/
|
|
+ EOF
|
|
+'
|
|
+
|
|
test_expect_success 'host-less URLs are parsed as empty host' '
|
|
check fill "verbatim foo bar" <<-\EOF
|
|
url=cert:///path/to/cert.pem
|