development/git
development/git
1
0
Fork 0
mirror of https://github.com/git/git synced 2024-09-12 21:04:12 +00:00
Code Issues Packages Projects Releases Wiki Activity

daemon: fix off-by-one in logging extended attributes

Browse source 
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>
This commit is contained in:
Jeff King 2018-01-24 19:56:07 -05:00 committed by Junio C Hamano
parent 314a73d658
commit 19136be3f8
2 changed files with 13 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
daemon.c
View file

@ -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--;

11
t/t5570-git-daemon.sh
View file

@ -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