@matheusbsbs / matheus-bagliano / commits / 447ac906e1

attr: fix out-of-bounds read with unreasonable amount of patterns

The `struct attr_stack` tracks the stack of all patterns together with their attributes. When parsing a gitattributes file that has more than 2^31 such patterns though we may trigger multiple out-of-bounds reads on 64 bit platforms. This is because while the `num_matches` variable is an unsigned integer, we always use a signed integer to iterate over them. I have not been able to reproduce this issue due to memory constraints on my systems. But despite the out-of-bounds reads, the worst thing that can seemingly happen is to call free(3P) with a garbage pointer when calling `attr_stack_free()`. Fix this bug by using unsigned integers to iterate over the array. While this makes the iteration somewhat awkward when iterating in reverse, it is at least better than knowingly running into an out-of-bounds read. While at it, convert the call to `ALLOC_GROW` to use `ALLOC_GROW_BY` instead. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Dec 1, 2022 at 15:45 UTC 447ac906e189535e77dcb1f4bbe3f1bc917d4c12
1 file changed +9 -9
attr.c
+9 -9
@@ -446,7 +446,7 @@ struct attr_stack {
446
447 static void attr_stack_free(struct attr_stack *e)
448 {
449 - int i;
449 + unsigned i;
450 free(e->origin);
451 for (i = 0; i < e->num_matches; i++) {
452 struct match_attr *a = e->attrs[i];
@@ -660,8 +660,8 @@ static void handle_attr_line(struct attr_stack *res,
660 a = parse_attr_line(line, src, lineno, macro_ok);
661 if (!a)
662 return;
663 - ALLOC_GROW(res->attrs, res->num_matches + 1, res->alloc);
664 - res->attrs[res->num_matches++] = a;
663 + ALLOC_GROW_BY(res->attrs, res->num_matches, 1, res->alloc);
664 + res->attrs[res->num_matches - 1] = a;
665 }
666
667 static struct attr_stack *read_attr_from_array(const char **list)
@@ -1025,11 +1025,11 @@ static int fill(const char *path, int pathlen, int basename_offset,
1025 struct all_attrs_item *all_attrs, int rem)
1026 {
1027 for (; rem > 0 && stack; stack = stack->prev) {
1028 - int i;
1028 + unsigned i;
1029 const char *base = stack->origin ? stack->origin : "";
1030
1031 - for (i = stack->num_matches - 1; 0 < rem && 0 <= i; i--) {
1032 - const struct match_attr *a = stack->attrs[i];
1031 + for (i = stack->num_matches; 0 < rem && 0 < i; i--) {
1032 + const struct match_attr *a = stack->attrs[i - 1];
1033 if (a->is_macro)
1034 continue;
1035 if (path_matches(path, pathlen, basename_offset,
@@ -1060,9 +1060,9 @@ static void determine_macros(struct all_attrs_item *all_attrs,
1060 const struct attr_stack *stack)
1061 {
1062 for (; stack; stack = stack->prev) {
1063 - int i;
1064 - for (i = stack->num_matches - 1; i >= 0; i--) {
1065 - const struct match_attr *ma = stack->attrs[i];
1063 + unsigned i;
1064 + for (i = stack->num_matches; i > 0; i--) {
1065 + const struct match_attr *ma = stack->attrs[i - 1];
1066 if (ma->is_macro) {
1067 int n = ma->u.attr->attr_nr;
1068 if (!all_attrs[n].macro) {