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

[PATCH 3/3] range-diff: use ssize_t for parsed "len" in read_patches()

From
Jeff King <peff@peff.net>
Date
Aug 9, 2021, 22:48 UTC
Message-ID
<YRGw0FH3XciRGq5x@coredump.intra.peff.net>
In-Reply-To
<YRGwGDvOte/o9vkd@coredump.intra.peff.net>

As we iterate through the buffer containing git-log output, parsing lines, we use an "int" to store the size of an individual line. This should be a size_t, as we have no guarantee that there is not a malicious 2GB+ commit-message line in the output.

Overflowing this integer probably doesn't do anything _too_ terrible. We are not using the value to size a buffer, so the worst case is probably an out-of-bounds read from before the array. But it's easy enough to fix.

Note that we have to use ssize_t here, since we also store the length result from parse_git_diff_header(), which may return a negative value for error. That function actually returns an int itself, which has a similar overflow problem, but I'll leave that for another day. Much of the apply.c code uses ints and should be converted as a whole; in the meantime, a negative return from parse_git_diff_header() will be interpreted as an error, and we'll bail (so we can't handle such a case, but given that it's likely to be malicious anyway, the important thing is we don't have any memory errors).

Signed-off-by: Jeff King <peff@peff.net>
---
 range-diff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/range-diff.c b/range-diff.c
index 088db1b1ce..e731525e66 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -38,7 +38,7 @@ static int read_patches(const char *range, struct string_list *list,
 	struct patch_util *util = NULL;
 	int in_header = 1;
 	char *line, *current_filename = NULL;
-	int len;
+	ssize_t len;
 	size_t size;
 
 	strvec_pushl(&cp.args, "log", "--no-color", "-p", "--no-merges",
-- 
2.33.0.rc1.475.g023efe0ae4
Previous: Jeff KingNext: Derrick Stolee
Message 4 of 6 in “some small range-diff read_patches() fixes”
  1. 0/3 some small range-diff read_patches() fixesJeff King, Aug 9, 2021
  2. 1/3 range-diff: drop useless "offset" variable from read_patches()Jeff King, Aug 9, 2021
  3. 2/3 range-diff: handle unterminated lines in read_patches()Jeff King, Aug 9, 2021
  4. 3/3 range-diff: use ssize_t for parsed "len" in read_patches()Jeff King, Aug 9, 2021
  5. Derrick StoleeAug 10, 2021
  6. Johannes SchindelinAug 14, 2021

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.