@matheusbsbs / matheus-bagliano / commits / 550fbcad1c

daemon: handle NULs in extended attribute string

If we receive a request with extended attributes after the NUL, we try to write those attributes to the log. We do so with a "%s" format specifier, which will only show characters up to the first NUL. That's enough for printing a "host=" specifier. But since dfe422d04d (daemon: recognize hidden request arguments, 2017-10-16) we may have another NUL, followed by protocol parameters, and those are not logged at all. Let's cut out the attempt to show the whole string, and instead log when we parse individual attributes. We could leave the "extended attributes (%d bytes) exist" part of the log, which in theory could alert us to attributes that fail to parse. But anything we don't parse as a "host=" parameter gets blindly added to the "protocol" attribute, so we'd see it in that part of the log. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jan 24, 2018 at 19:56 UTC 550fbcad1c464df9e32cab15a8c6a01f91b1629c

2 files changed +9 -8

daemon.c

+4 -5

@@ -597,6 +597,7 @@ static char *parse_host_arg(struct hostinfo *hi, char *extra_args, int buflen)

597 if (strncasecmp("host=", extra_args, 5) == 0) {

598 val = extra_args + 5;

599 vallen = strlen(val) + 1;

600 + loginfo("Extended attribute \"host\": %s", val);

601 if (*val) {

602 /* Split <host>:<port> at colon. */

603 char *host;

@@ -647,9 +648,11 @@ static void parse_extra_args(struct hostinfo *hi, struct argv_array *env,

648 }

649 }

650

650 - if (git_protocol.len > 0)

651 + if (git_protocol.len > 0) {

652 + loginfo("Extended attribute \"protocol\": %s", git_protocol.buf);

653 argv_array_pushf(env, GIT_PROTOCOL_ENVIRONMENT "=%s",

654 git_protocol.buf);

655 + }

656 strbuf_release(&git_protocol);

657 }

658

@@ -757,10 +760,6 @@ static int execute(void)

760 alarm(0);

761

762 len = strlen(line);

760 - if (pktlen != len)

761 - loginfo("Extended attributes (%d bytes) exist <%.*s>",

762 - (int) pktlen - len - 1,

763 - (int) pktlen - len - 1, line + len + 1);

763 if (len && line[len-1] == '\n') {

764 line[--len] = 0;

765 pktlen--;

t/t5570-git-daemon.sh

+5 -3

@@ -183,13 +183,15 @@ test_expect_success 'hostname cannot break out of directory' '

183 git ls-remote "$GIT_DAEMON_URL/escape.git"

184 '

185

186 -test_expect_success 'daemon log records hostnames' '

186 +test_expect_success 'daemon log records all attributes' '

187 cat >expect <<-\EOF &&

188 - Extended attributes (15 bytes) exist <host=localhost>

188 + Extended attribute "host": localhost

189 + Extended attribute "protocol": version=1

190 EOF

191 >daemon.log &&

192 GIT_OVERRIDE_VIRTUAL_HOST=localhost \

192 - git ls-remote "$GIT_DAEMON_URL/interp.git" &&

193 + git -c protocol.version=1 \

194 + ls-remote "$GIT_DAEMON_URL/interp.git" &&

195 grep -i extended.attribute daemon.log | cut -d" " -f2- >actual &&

196 test_cmp expect actual

197 '