Commit f647bbc88d243a81d8771ba2fd1c346c34a3d9d7
2018-10-29T17:25:09
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.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55
diff --git a/src/tree.c b/src/tree.c
index d628aeb..9772f5a 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 6ac3a2d..67b7586 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;