summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJeff King <peff@peff.net>2020-03-11 21:53:41 (GMT)
committerJeff King <peff@peff.net>2020-03-12 06:55:16 (GMT)
commit9a6bbee8006c24b46a85d29e7b38cfa79e9ab21b (patch)
tree69d12720e68f9cba226bf4ffcc0ca3a668de2c72
parenta5ab8d03173458b76b8452efd90a7173f490c132 (diff)
downloadgit-9a6bbee8006c24b46a85d29e7b38cfa79e9ab21b.zip
git-9a6bbee8006c24b46a85d29e7b38cfa79e9ab21b.tar.gz
git-9a6bbee8006c24b46a85d29e7b38cfa79e9ab21b.tar.bz2
credential: avoid writing values with newlines
The credential protocol that we use to speak to helpers can't represent values with newlines in them. This was an intentional design choice to keep the protocol simple, since none of the values we pass should generally have newlines. However, if we _do_ encounter a newline in a value, we blindly transmit it in credential_write(). Such values may break the protocol syntax, or worse, inject new valid lines into the protocol stream. The most likely way for a newline to end up in a credential struct is by decoding a URL with a percent-encoded newline. However, since the bug occurs at the moment we write the value to the protocol, we'll catch it there. That should leave no possibility of accidentally missing a code path that can trigger the problem. At this level of the code we have little choice but to die(). However, since we'd not ever expect to see this case outside of a malicious URL, that's an acceptable outcome. Reported-by: Felix Wilhelm <fwilhelm@google.com>
-rw-r--r--credential.c2
-rwxr-xr-xt/t0300-credentials.sh6
2 files changed, 8 insertions, 0 deletions
diff --git a/credential.c b/credential.c
index 9747f47..00ee4d6 100644
--- a/credential.c
+++ b/credential.c
@@ -194,6 +194,8 @@ static void credential_write_item(FILE *fp, const char *key, const char *value)
{
if (!value)
return;
+ if (strchr(value, '\n'))
+ die("credential value for %s contains newline", key);
fprintf(fp, "%s=%s\n", key, value);
}
diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index 03bd31e..15cc3c5 100755
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -309,4 +309,10 @@ test_expect_success 'empty helper spec resets helper list' '
EOF
'
+test_expect_success 'url parser rejects embedded newlines' '
+ test_must_fail git credential fill <<-\EOF
+ url=https://one.example.com?%0ahost=two.example.com/
+ EOF
+'
+
test_done