62 lines
2.0 KiB
Diff
62 lines
2.0 KiB
Diff
From 10372307c11d0128c40d730418d3776f1a959ce3 Mon Sep 17 00:00:00 2001
|
|
From: Jeff King <peff@peff.net>
|
|
Date: Wed, 11 Mar 2020 17:53:41 -0400
|
|
Subject: [PATCH] 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>
|
|
---
|
|
credential.c | 2 ++
|
|
t/t0300-credentials.sh | 6 ++++++
|
|
2 files changed, 8 insertions(+)
|
|
|
|
diff --git a/credential.c b/credential.c
|
|
index 62be651..a79aff0 100644
|
|
--- a/credential.c
|
|
+++ b/credential.c
|
|
@@ -195,6 +195,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 82eaaea..26f3c3a 100755
|
|
--- a/t/t0300-credentials.sh
|
|
+++ b/t/t0300-credentials.sh
|
|
@@ -308,4 +308,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
|
|
--
|
|
1.8.3.1
|
|
|