| From a2d973517c7c2558fdbbb75c4fe0f435c7599bb6 Mon Sep 17 00:00:00 2001 |
| From: Jeff King <peff@peff.net> |
| Date: Wed, 11 Mar 2020 17:53:41 -0400 |
| Subject: 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> |
| (cherry picked from commit 9a6bbee8006c24b46a85d29e7b38cfa79e9ab21b) |
| Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> |
| --- |
| credential.c | 2 ++ |
| t/t0300-credentials.sh | 6 ++++++ |
| 2 files changed, 8 insertions(+) |
| |
| diff --git a/credential.c b/credential.c |
| index 62be651b03..a79aff0c9c 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 82eaaea0f4..26f3c3a972 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 |
| -- |
| 2.26.0.292.g33ef6b2f38 |
| |
