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

[PATCH 2/3] remote-curl: verify smart-http metadata lines

From
Jeff King <peff@peff.net>
Date
Feb 16, 2013, 06:47 UTC
Message-ID
<20130216064707.GB22626@sigill.intra.peff.net>
In-Reply-To
<20130216064455.GA27063@sigill.intra.peff.net>

A smart http ref advertisement starts with a packet containing the service header, followed by an arbitrary number of packets containing other metadata headers, followed by a flush packet.

We don't currently recognize any other metadata headers, so we just parse through any extra packets, throwing away their contents. However, we don't do so very carefully, and just stop at the first error or flush packet.

Let's flag any errors we see here, which might be a sign of truncated or corrupted output. Since the rest of the data should be the ref advertisement, and since we pass that along to our helper programs (like fetch-pack), they will probably notice the error, as whatever cruft is in the buffer will not parse. However, it's nice to note problems as early as possible, which can help in debugging the root cause.

Signed-off-by: Jeff King <peff@peff.net>
---
 remote-curl.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index 933c69a..73134f5 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -90,6 +90,17 @@ static void free_discovery(struct discovery *d)
 	}
 }
 
+static int read_packets_until_flush(char **buf, size_t *len)
+{
+	while (1) {
+		int r = packet_get_line(NULL, buf, len);
+		if (r < 0)
+			return -1;
+		if (r == 0)
+			return 0;
+	}
+}
+
 static struct discovery* discover_refs(const char *service)
 {
 	struct strbuf exp = STRBUF_INIT;
@@ -155,11 +166,13 @@ static struct discovery* discover_refs(const char *service)
 
 		/* The header can include additional metadata lines, up
 		 * until a packet flush marker.  Ignore these now, but
-		 * in the future we might start to scan them.
+		 * in the future we might start to scan them. However, we do
+		 * still check to make sure we are getting valid packet lines,
+		 * ending with a flush.
 		 */
-		strbuf_reset(&buffer);
-		while (packet_get_line(&buffer, &last->buf, &last->len) > 0)
-			strbuf_reset(&buffer);
+		if (read_packets_until_flush(&last->buf, &last->len) < 0)
+			die("smart-http metadata lines are invalid at %s",
+			    refs_url);
 
 		last->proto_git = 1;
 	}
-- 
1.8.1.2.11.g1a2f572
Previous: Jonathan NiederNext: Jonathan Nieder
Message 4 of 50 in “make smart-http more robust against bogus server input”
  1. 0/3 make smart-http more robust against bogus server inputJeff King, Feb 16, 2013
  2. 1/3 pkt-line: teach packet_get_line a no-op modeJeff King, Feb 16, 2013
  3. Jonathan NiederFeb 17, 2013
  4. 2/3 remote-curl: verify smart-http metadata linesJeff King, Feb 16, 2013
  5. Jonathan NiederFeb 17, 2013
  6. Jeff KingFeb 17, 2013
  7. Jonathan NiederFeb 18, 2013
  8. Jeff KingFeb 18, 2013
  9. 3/3 remote-curl: sanity check ref advertisement from serverJeff King, Feb 16, 2013
  10. Jonathan NiederFeb 17, 2013
  11. Jeff KingFeb 17, 2013
  12. Jonathan NiederFeb 18, 2013
  13. 0/10 pkt-line and remote-curl cleanups serverJeff King, Feb 18, 2013
  14. 01/10 pkt-line: move a misplaced commentJeff King, Feb 18, 2013
  15. 02/10 pkt-line: drop safe_write functionJeff King, Feb 18, 2013
  16. Jonathan NiederFeb 18, 2013
  17. Jeff KingFeb 18, 2013
  18. 03/10 pkt-line: clean up "gentle" reading functionJeff King, Feb 18, 2013
  19. Jonathan NiederFeb 18, 2013
  20. Jeff KingFeb 18, 2013
  21. 04/10 pkt-line: change error message for oversized packetJeff King, Feb 18, 2013
  22. Junio C HamanoFeb 18, 2013
  23. Jeff KingFeb 18, 2013
  24. Junio C HamanoFeb 18, 2013
  25. Jeff KingFeb 18, 2013
  26. Jeff KingFeb 20, 2013
  27. Junio C HamanoFeb 20, 2013
  28. Jonathan NiederFeb 18, 2013
  29. Jeff KingFeb 18, 2013
  30. 05/10 pkt-line: rename s/packet_read_line/packet_read/Jeff King, Feb 18, 2013
  31. Jonathan NiederFeb 18, 2013
  32. Jeff KingFeb 18, 2013
  33. Jonathan NiederFeb 18, 2013
  34. 06/10 pkt-line: share buffer/descriptor reading implementationJeff King, Feb 18, 2013
  35. Jonathan NiederFeb 18, 2013
  36. Jeff KingFeb 18, 2013
  37. Jonathan NiederFeb 18, 2013
  38. 07/10 teach get_remote_heads to read from a memory bufferJeff King, Feb 18, 2013
  39. 08/10 remote-curl: pass buffer straight to get_remote_headsJeff King, Feb 18, 2013
  40. Jonathan NiederFeb 18, 2013
  41. Junio C HamanoFeb 18, 2013
  42. Jeff KingFeb 18, 2013
  43. Junio C HamanoFeb 18, 2013
  44. Jeff KingFeb 18, 2013
  45. Shawn PearceFeb 20, 2013
  46. 09/10 remote-curl: move ref-parsing code up in fileJeff King, Feb 18, 2013
  47. 10/10 remote-curl: always parse incoming refsJeff King, Feb 18, 2013
  48. Jonathan NiederFeb 18, 2013
  49. Shawn PearceFeb 20, 2013
  50. Shawn PearceFeb 20, 2013

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.