tree: fix mode parsing reading out-of-bounds

When parsing a tree entry's mode, we will eagerly parse until we hit a
character that is not in the accepted set of octal digits '0' - '7'. If
the provided buffer is not a NUL terminated one, we may thus read
out-of-bounds.

Fix the issue by passing the buffer length to `parse_mode` and paying
attention to it. Note that this is not a vulnerability in our usual code
paths, as all object data read from the ODB is NUL terminated.

2 files changed, 16 insertions, 3 deletions

diff --git a/src/tree.c b/src/tree.c
index d628aeb64..9772f5a2d 100644
--- a/src/tree.c
+++ b/src/tree.c
@@ -356,15 +356,16 @@ static int tree_error(const char *str, const char *path)
 	return -1;
 }
 
-static int parse_mode(unsigned int *modep, const char *buffer, const char **buffer_out)
+static int parse_mode(unsigned int *modep, const char *buffer, size_t buffer_len, const char **buffer_out)
 {
+	const char *buffer_end = buffer + buffer_len;
 	unsigned char c;
 	unsigned int mode = 0;
 
 	if (*buffer == ' ')
 		return -1;
 
-	while ((c = *buffer++) != ' ') {
+	while (buffer < buffer_end && (c = *buffer++) != ' ') {
 		if (c < '0' || c > '7')
 			return -1;
 		mode = (mode << 3) + (c - '0');
@@ -394,7 +395,7 @@ int git_tree__parse_raw(void *_tree, const char *data, size_t size)
 		const char *nul;
 		unsigned int attr;
 
-		if (parse_mode(&attr, buffer, &buffer) < 0 || !buffer)
+		if (parse_mode(&attr, buffer, buffer_end - buffer, &buffer) < 0 || !buffer)
 			return tree_error("failed to parse tree: can't parse filemode", NULL);
 
 		if ((nul = memchr(buffer, 0, buffer_end - buffer)) == NULL)
diff --git a/tests/object/tree/parse.c b/tests/object/tree/parse.c
index 6ac3a2d00..67b758667 100644
--- a/tests/object/tree/parse.c
+++ b/tests/object/tree/parse.c
@@ -109,6 +109,18 @@ void test_object_tree_parse__missing_mode_fails(void)
 	assert_tree_fails(data, ARRAY_SIZE(data) - 1);
 }
 
+void test_object_tree_parse__mode_doesnt_cause_oob_read(void)
+{
+	const char data[] = "100644 bar\x00" OID1_HEX;
+	assert_tree_fails(data, 2);
+	/*
+	 * An oob-read would correctly parse the filename and
+	 * later fail to parse the OID with a different error
+	 * message
+	 */
+	cl_assert(strstr(giterr_last()->message, "object is corrupted"));
+}
+
 void test_object_tree_parse__missing_filename_separator_fails(void)
 {
 	const char data[] = "100644bar\x00" OID1_HEX;