diff options
author | Taylor Blau <me@ttaylorr.com> | 2023-05-01 11:54:06 -0400 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2023-05-01 09:27:02 -0700 |
commit | 0a3a972c163b2c5ed81a8e2c12fbf0c53eeb210c (patch) | |
tree | 924338f51518ebfc62c87a9af5c52602ccbc1b00 /thread-utils.h | |
parent | 64f1e658e935bea6c9afdc4fa8be1d3ad6740355 (diff) | |
download | git-0a3a972c163b2c5ed81a8e2c12fbf0c53eeb210c.tar.gz |
contrib/credential: embiggen fixed-size buffer in wincred
As in previous commits, harden the wincred credential helper against the
aforementioned protocol injection attack.
Unlike the approached used for osxkeychain and libsecret, where a
fixed-size buffer was replaced with `getline()`, we must take a
different approach here. There is no `getline()` equivalent in Windows,
and the function is not available to us with ordinary compiler settings.
Instead, allocate a larger (still fixed-size) buffer in which to process
each line. The value of 100 KiB is chosen to match the maximum-length
header that curl will allow, CURL_MAX_HTTP_HEADER.
To ensure that we are reading complete lines at a time, and that we
aren't susceptible to a similar injection attack (albeit with more
padding), ensure that each read terminates at a newline (i.e., that no
line is more than 100 KiB long).
Note that it isn't sufficient to turn the old loop into something like:
while (len && strchr("\r\n", buf[len - 1])) {
buf[--len] = 0;
ends_in_newline = 1;
}
because if an attacker sends something like:
[aaaaa.....]\r
host=example.com\r\n
the credential helper would fill its buffer after reading up through the
first '\r', call fgets() again, and then see "host=example.com\r\n" on
its line.
Note that the original code was written in a way that would trim an
arbitrary number of "\r" and "\n" from the end of the string. We should
get only a single "\n" (since the point of `fgets()` is to return the
buffer to us when it sees one), and likewise would not expect to see
more than one associated "\r". The new code trims a single "\r\n", which
matches the original intent.
[1]: https://curl.se/libcurl/c/CURLOPT_HEADERFUNCTION.html
Tested-by: Matthew John Cheetham <mjcheetham@outlook.com>
Helped-by: Matthew John Cheetham <mjcheetham@outlook.com>
Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'thread-utils.h')
0 files changed, 0 insertions, 0 deletions