git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 4/6] daemon: handle NULs in extended attribute string

From
Jeff King <peff@peff.net>
Date
Jan 25, 2018, 00:56 UTC
Message-ID
<20180125005619.GD26850@sigill.intra.peff.net>
In-Reply-To
<20180125005447.GA26661@sigill.intra.peff.net>

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>
---
 daemon.c              | 9 ++++-----
 t/t5570-git-daemon.sh | 8 +++++---
 2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/daemon.c b/daemon.c
index d78afc8e96..652f89b6e7 100644
--- a/daemon.c
+++ b/daemon.c
@@ -597,6 +597,7 @@ static char *parse_host_arg(struct hostinfo *hi, char *extra_args, int buflen)
 		if (strncasecmp("host=", extra_args, 5) == 0) {
 			val = extra_args + 5;
 			vallen = strlen(val) + 1;
+			loginfo("Extended attribute \"host\": %s", val);
 			if (*val) {
 				/* Split <host>:<port> at colon. */
 				char *host;
@@ -647,9 +648,11 @@ static void parse_extra_args(struct hostinfo *hi, struct argv_array *env,
 		}
 	}
 
-	if (git_protocol.len > 0)
+	if (git_protocol.len > 0) {
+		loginfo("Extended attribute \"protocol\": %s", git_protocol.buf);
 		argv_array_pushf(env, GIT_PROTOCOL_ENVIRONMENT "=%s",
 				 git_protocol.buf);
+	}
 	strbuf_release(&git_protocol);
 }
 
@@ -757,10 +760,6 @@ static int execute(void)
 	alarm(0);
 
 	len = strlen(line);
-	if (pktlen != len)
-		loginfo("Extended attributes (%d bytes) exist <%.*s>",
-			(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 359af3994a..b556469db6 100755
--- a/t/t5570-git-daemon.sh
+++ b/t/t5570-git-daemon.sh
@@ -183,13 +183,15 @@ test_expect_success 'hostname cannot break out of directory' '
 		git ls-remote "$GIT_DAEMON_URL/escape.git"
 '
 
-test_expect_success 'daemon log records hostnames' '
+test_expect_success 'daemon log records all attributes' '
 	cat >expect <<-\EOF &&
-	Extended attributes (15 bytes) exist <host=localhost>
+	Extended attribute "host": localhost
+	Extended attribute "protocol": version=1
 	EOF
 	>daemon.log &&
 	GIT_OVERRIDE_VIRTUAL_HOST=localhost \
-		git ls-remote "$GIT_DAEMON_URL/interp.git" &&
+		git -c protocol.version=1 \
+			ls-remote "$GIT_DAEMON_URL/interp.git" &&
 	grep -i extended.attribute daemon.log | cut -d" " -f2- >actual &&
 	test_cmp expect actual
 '
-- 
2.16.1.273.gfdaa03aa74
Previous: Jeff KingNext: Jeff King
Message 7 of 13 in “off-by-one errors in git-daemon”
  1. 0/6 off-by-one errors in git-daemonJeff King, Jan 25, 2018
  2. 1/6 t5570: use ls-remote instead of clone for interp testsJeff King, Jan 25, 2018
  3. 2/6 t/lib-git-daemon: record daemon logJeff King, Jan 25, 2018
  4. Lucas WerkmeisterJan 25, 2018
  5. Jeff KingJan 25, 2018
  6. 3/6 daemon: fix off-by-one in logging extended attributesJeff King, Jan 25, 2018
  7. 4/6 daemon: handle NULs in extended attribute stringJeff King, Jan 25, 2018
  8. 5/6 t/lib-git-daemon: add network-protocol helpersJeff King, Jan 25, 2018
  9. 6/6 daemon: fix length computation in newline strippingJeff King, Jan 25, 2018
  10. Eric SunshineJan 25, 2018
  11. Jeff KingJan 26, 2018
  12. Junio C HamanoJan 25, 2018
  13. Jeff KingJan 25, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.