undef/git-commit-vandalism
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

daemon: fix length computation in newline stripping

Browse Source 
When git-daemon gets a pktline request, we strip off any
trailing newline, replacing it with a NUL. Clients prior to
5ad312bede (in git v1.4.0) would send:

  git-upload-pack repo.git\n

and we need to strip it off to understand their request.
After 5ad312bede, we send the host attribute but no newline,
like:

  git-upload-pack repo.git\0host=example.com\0

Both of these are parsed correctly by git-daemon. But if
some client were to combine the two:

  git-upload-pack repo.git\n\0host=example.com\0

we don't parse it correctly. The problem is that we use the
"len" variable to record the position of the NUL separator,
but then decrement it when we strip the newline. So we start
with:

  git-upload-pack repo.git\n\0host=example.com\0
                             ^-- len

and end up with:

  git-upload-pack repo.git\0\0host=example.com\0
                           ^-- len

This is arguably correct, since "len" tells us the length of
the initial string, but we don't actually use it for that.
What we do use it for is finding the offset of the extended
attributes; they used to be at len+1, but are now at len+2.

We can solve that by just leaving "len" where it is. We
don't have to care about the length of the shortened string,
since we just treat it like a C string.

No version of Git ever produced such a string, but it seems
like the daemon code meant to handle this case (and it seems
like a reasonable thing for somebody to do in a 3rd-party
implementation).

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:58:54 -05:00 committed by Junio C Hamano
parent 4414a15002
commit ed15e58efe
2 changed files with 17 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
daemon.c
View File

@ -760,10 +760,8 @@ static int execute(void)
alarm(0);
len = strlen(line);
if (len && line[len-1] == '\n') {
line[--len] = 0;
pktlen--;
}
if (len && line[len-1] == '\n')
line[len-1] = 0;
/* parse additional args hidden behind a NUL byte */
if (len != pktlen)

15
t/t5570-git-daemon.sh
View File

@ -196,5 +196,20 @@ test_expect_success 'daemon log records all attributes' '
test_cmp expect actual
'
test_expect_success FAKENC 'hostname interpolation works after LF-stripping' '
{
printf "git-upload-pack /interp.git\n\0host=localhost" | packetize
printf "0000"
} >input &&
fake_nc "$GIT_DAEMON_HOST_PORT" <input >output &&
depacketize <output >output.raw &&
# just pick out the value of master, which avoids any protocol
# particulars
perl -lne "print \$1 if m{^(\\S+) refs/heads/master}" <output.raw >actual &&
git -C "$repo" rev-parse master >expect &&
test_cmp expect actual
'
stop_git_daemon
test_done