Skip to content

Commit 447ac90

Browse files
pks-tgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 34ace8b commit 447ac90

File tree

1 file changed

+9
-9
lines changed

1 file changed

+9
-9
lines changed

attr.c

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,7 @@ struct attr_stack {
446446

447447
static void attr_stack_free(struct attr_stack *e)
448448
{
449-
int i;
449+
unsigned i;
450450
free(e->origin);
451451
for (i = 0; i < e->num_matches; i++) {
452452
struct match_attr *a = e->attrs[i];
@@ -660,8 +660,8 @@ static void handle_attr_line(struct attr_stack *res,
660660
a = parse_attr_line(line, src, lineno, macro_ok);
661661
if (!a)
662662
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;
665665
}
666666

667667
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,
10251025
struct all_attrs_item *all_attrs, int rem)
10261026
{
10271027
for (; rem > 0 && stack; stack = stack->prev) {
1028-
int i;
1028+
unsigned i;
10291029
const char *base = stack->origin ? stack->origin : "";
10301030

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];
10331033
if (a->is_macro)
10341034
continue;
10351035
if (path_matches(path, pathlen, basename_offset,
@@ -1060,9 +1060,9 @@ static void determine_macros(struct all_attrs_item *all_attrs,
10601060
const struct attr_stack *stack)
10611061
{
10621062
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];
10661066
if (ma->is_macro) {
10671067
int n = ma->u.attr->attr_nr;
10681068
if (!all_attrs[n].macro) {

0 commit comments

Comments
 (0)