@matheusbsbs / matheus-bagliano / commits / ca63af0a24

reftable/stack: fix parameter validation when compacting range

The `stack_compact_range()` function receives a "first" and "last" index that indicates which tables of the reftable stack should be compacted. Naturally, "first" must be smaller than "last" in order to identify a proper range of tables to compress, which we indeed also assert in the function. But the validations happens after we have already allocated arrays with a size of `last - first + 1`, leading to an underflow and thus an invalid allocation size. Fix this by reordering the array allocations to happen after we have validated parameters. While at it, convert the array allocations to use the newly introduced macros. Note that the relevant variables pointing into arrays should also be converted to use `size_t` instead of `int`. This is left for a later commit in this series. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Feb 6, 2024 at 07:35 UTC ca63af0a248d31bf917d207722b284da41ffcee7

1 file changed +13 -11

reftable/stack.c

+13 -11

@@ -966,6 +966,7 @@ done:

966 static int stack_compact_range(struct reftable_stack *st, int first, int last,

967 struct reftable_log_expiry_config *expiry)

968 {

969 + char **delete_on_success = NULL, **subtable_locks = NULL, **listp = NULL;

970 struct strbuf temp_tab_file_name = STRBUF_INIT;

971 struct strbuf new_table_name = STRBUF_INIT;

972 struct strbuf lock_file_name = STRBUF_INIT;

@@ -974,12 +975,7 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last,

975 int err = 0;

976 int have_lock = 0;

977 int lock_file_fd = -1;

977 - int compact_count = last - first + 1;

978 - char **listp = NULL;

979 - char **delete_on_success =

980 - reftable_calloc(compact_count + 1, sizeof(*delete_on_success));

981 - char **subtable_locks =

982 - reftable_calloc(compact_count + 1, sizeof(*subtable_locks));

978 + int compact_count;

979 int i = 0;

980 int j = 0;

981 int is_empty_table = 0;

@@ -989,6 +985,10 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last,

985 goto done;

986 }

987

988 + compact_count = last - first + 1;

989 + REFTABLE_CALLOC_ARRAY(delete_on_success, compact_count + 1);

990 + REFTABLE_CALLOC_ARRAY(subtable_locks, compact_count + 1);

991 +

992 st->stats.attempts++;

993

994 strbuf_reset(&lock_file_name);

@@ -1146,12 +1146,14 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last,

1146 done:

1147 free_names(delete_on_success);

1148

1149 - listp = subtable_locks;

1150 - while (*listp) {

1151 - unlink(*listp);

1152 - listp++;

1149 + if (subtable_locks) {

1150 + listp = subtable_locks;

1151 + while (*listp) {

1152 + unlink(*listp);

1153 + listp++;

1154 + }

1155 + free_names(subtable_locks);

1156 }

1154 - free_names(subtable_locks);

1157 if (lock_file_fd >= 0) {

1158 close(lock_file_fd);

1159 lock_file_fd = -1;