[PATCH 2/7] t/lib-credential.sh: ensure credential helpers handle long headers

Add a test ensuring that the "wwwauth[]" field cannot be used to inject malicious data into the credential helper stream.

Many of the credential helpers in contrib/credential read the newline-delimited protocol stream one line at a time by repeatedly calling fgets() into a fixed-size buffer.

This assumes that each line is no more than 1024 characters long, since each iteration of the loop assumes that it is parsing starting at the beginning of a new line in the stream. However, similar to a5bb10fd5e (config: avoid fixed-sized buffer when renaming/deleting a section, 2023-04-06), if a line is longer than 1024 characters, a malicious actor can embed another command within an existing line, bypassing the usual checks introduced in 9a6bbee800 (credential: avoid writing values with newlines, 2020-03-11).

As with the problem fixed in that commit, specially crafted input can cause the helper to return the credential for the wrong host, letting an attacker trick the victim into sending credentials for one host to another.

Luckily, all parts of the credential helper protocol that are available in a tagged release of Git are immune to this attack:

  - "protocol" is restricted to known values, and is thus immune.

  - "host" is immune because curl will reject hostnames that have a '='
    character in them, which would be required to carry out this attack.

  - "username" is immune, because the buffer characters to fill out the
    first `fgets()` call would pollute the `username` field, causing the
    credential helper to return nothing (because it would match a
    username if present, and the username of the credential to be stolen
    is likely not 1024 characters).

  - "password" is immune because providing a password instructs
    credential helpers to avoid filling credentials in the first place.

  - "path" is similar to username; if present, it is not likely to match
    any credential the victim is storing. It's also not enabled by
    default; the victim would have to set credential.useHTTPPath
    explicitly.

However, the new "wwwauth[]" field introduced via 5f2117b24f (credential: add WWW-Authenticate header to cred requests, 2023-02-27) can be used to inject data into the credential helper stream. For example, running:

    {
      printf 'HTTP/1.1 401\r\n'
      printf 'WWW-Authenticate: basic realm='
      perl -e 'print "a" x 1024'
      printf 'host=victim.com\r\n'
    } | nc -Nlp 8080

in one terminal, and then:

    git clone http://localhost:8080

in another would result in a line like:

    wwwauth[]=basic realm=aaa[...]aaahost=victim.com

being sent to the credential helper. If we tweak that "1024" to align our output with the helper's buffer size and the rest of the data on the line, it can cause the helper to see "host=victim.com" on its own line, allowing motivated attackers to exfiltrate credentials belonging to "victim.com".

The below test demonstrates these failures and provides us with a test to ensure that our fix is correct. That said, it has a couple of shortcomings:

  - it's in t0303, since that's the only mechanism we have for testing
    random helpers. But that means nobody is going to run it under
    normal circumstances.

  - to get the attack right, it has to line up the stuffed name with the
    buffer size, so we depend on the exact buffer size. I parameterized
    it so it could be used to test other helpers, but in practice it's
    not likely for anybody to do that.

Still, it's the best we can do, and will help us confirm the presence of the problem (and our fixes) in the new few patches.

Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 t/lib-credential.sh | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)