diff options
author | Jeff King <peff@peff.net> | 2018-01-24 19:56:07 -0500 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2018-01-25 13:50:17 -0800 |
commit | 19136be3f874ac265195ef35a8c5ed6c417eaea2 (patch) | |
tree | 886c23399e0c8f806aa1e21cbb80b1045e6e0f79 | |
parent | 314a73d6585bf9caecac3243f7de2d7330e1ff9f (diff) | |
download | git-19136be3f874ac265195ef35a8c5ed6c417eaea2.tar.gz |
daemon: fix off-by-one in logging extended attributes
If receive a request like:
git-upload-pack /foo.git\0host=localhost
we mark the offset of the NUL byte as "len", and then log
the bytes after the NUL with a "%.*s" placeholder, using
"pktlen - len" as the length, and "line + len + 1" as the
start of the string.
This is off-by-one, since the start of the string skips past
the separating NUL byte, but the adjusted length includes
it. Fortunately this doesn't actually read past the end of
the buffer, since "%.*s" will stop when it hits a NUL. And
regardless of what is in the buffer, packet_read() will
always add an extra NUL terminator for safety.
As an aside, the git.git client sends an extra NUL after a
"host" field, too, so we'd generally hit that one first, not
the one added by packet_read(). You can see this in the test
output which reports 15 bytes, even though the string has
only 14 bytes of visible data. But the point is that even a
client sending unusual data could not get us to read past
the end of the buffer, so this is purely a cosmetic fix.
Reported-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
-rw-r--r-- | daemon.c | 4 | ||||
-rwxr-xr-x | t/t5570-git-daemon.sh | 11 |
2 files changed, 13 insertions, 2 deletions
@@ -759,8 +759,8 @@ static int execute(void) len = strlen(line); if (pktlen != len) loginfo("Extended attributes (%d bytes) exist <%.*s>", - (int) pktlen - len, - (int) pktlen - len, line + len + 1); + (int) pktlen - len - 1, + (int) pktlen - len - 1, line + len + 1); if (len && line[len-1] == '\n') { line[--len] = 0; pktlen--; diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh index f92ebc5cd5..359af3994a 100755 --- a/t/t5570-git-daemon.sh +++ b/t/t5570-git-daemon.sh @@ -183,5 +183,16 @@ test_expect_success 'hostname cannot break out of directory' ' git ls-remote "$GIT_DAEMON_URL/escape.git" ' +test_expect_success 'daemon log records hostnames' ' + cat >expect <<-\EOF && + Extended attributes (15 bytes) exist <host=localhost> + EOF + >daemon.log && + GIT_OVERRIDE_VIRTUAL_HOST=localhost \ + git ls-remote "$GIT_DAEMON_URL/interp.git" && + grep -i extended.attribute daemon.log | cut -d" " -f2- >actual && + test_cmp expect actual +' + stop_git_daemon test_done |