Skip to content

Commit 19136be

Browse files
peffgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 314a73d commit 19136be

File tree

2 files changed

+13
-2
lines changed

2 files changed

+13
-2
lines changed

daemon.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -759,8 +759,8 @@ static int execute(void)
759759
len = strlen(line);
760760
if (pktlen != len)
761761
loginfo("Extended attributes (%d bytes) exist <%.*s>",
762-
(int) pktlen - len,
763-
(int) pktlen - len, line + len + 1);
762+
(int) pktlen - len - 1,
763+
(int) pktlen - len - 1, line + len + 1);
764764
if (len && line[len-1] == '\n') {
765765
line[--len] = 0;
766766
pktlen--;

t/t5570-git-daemon.sh

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,5 +183,16 @@ test_expect_success 'hostname cannot break out of directory' '
183183
git ls-remote "$GIT_DAEMON_URL/escape.git"
184184
'
185185

186+
test_expect_success 'daemon log records hostnames' '
187+
cat >expect <<-\EOF &&
188+
Extended attributes (15 bytes) exist <host=localhost>
189+
EOF
190+
>daemon.log &&
191+
GIT_OVERRIDE_VIRTUAL_HOST=localhost \
192+
git ls-remote "$GIT_DAEMON_URL/interp.git" &&
193+
grep -i extended.attribute daemon.log | cut -d" " -f2- >actual &&
194+
test_cmp expect actual
195+
'
196+
186197
stop_git_daemon
187198
test_done

0 commit comments

Comments
 (0)