Skip to content

arraylist: reject negative indices and size overflow #184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
prownd wants to merge 1 commit into
base: main
Choose a base branch
from
Open

arraylist: reject negative indices and size overflow #184

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions arraylist.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
# include <strings.h>
#endif /* HAVE_STRINGS_H */

#include <limits.h>

#include "arraylist.h"

struct array_list*
Expand Down Expand Up @@ -51,29 +53,42 @@ array_list_free(struct array_list *arr)
void*
array_list_get_idx(struct array_list *arr, int i)
{
if(i >= arr->length) return NULL;
if(i < 0 || i >= arr->length) return NULL;
return arr->array[i];
}

/* the largest element count whose byte size fits in an int-sized index space */
#define ARRAY_LIST_MAX_ELEMS ((int)(INT_MAX / sizeof(void*)))

static int array_list_expand_internal(struct array_list *arr, int max)
{
void *t;
int new_size;

if(max < arr->size) return 0;
new_size = arr->size << 1;
if (new_size < max)
/* Refuse sizes that would overflow when doubled or when converted to a
* byte count. */
if(max > ARRAY_LIST_MAX_ELEMS) return -1;
if(arr->size > ARRAY_LIST_MAX_ELEMS / 2) {
new_size = max;
if(!(t = realloc(arr->array, new_size*sizeof(void*)))) return -1;
} else {
new_size = arr->size << 1;
if (new_size < max)
new_size = max;
}
if(!(t = realloc(arr->array, (size_t)new_size*sizeof(void*)))) return -1;
arr->array = (void**)t;
(void)memset(arr->array + arr->size, 0, (new_size-arr->size)*sizeof(void*));
(void)memset(arr->array + arr->size, 0, (size_t)(new_size-arr->size)*sizeof(void*));
arr->size = new_size;
return 0;
}

int
array_list_put_idx(struct array_list *arr, int idx, void *data)
{
/* reject a negative index (heap underflow) and an index whose +1 would
* overflow int */
if(idx < 0 || idx == INT_MAX) return -1;

@cubic-dev-ai cubic-dev-ai Bot Sep 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The PR fixes the sanitizer-flagged OOB read/write and index+1 overflow, but adds no regression tests for the new guards. tests/test_array_api.c already covers put_idx/get_idx extensively yet never exercises a negative index (get_idx should return NULL, put_idx should return -1), idx == INT_MAX (put_idx should return -1), or the expansion cap — so nothing in make check would catch a reversion of these fixes. Add CHK assertions for these cases in test_array_api.c (e.g. fjson_object_array_get_idx(arr, -1) == NULL and fjson_object_array_put_idx(arr, -1, ...) == -1).

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At arraylist.c, line 91:

<comment>The PR fixes the sanitizer-flagged OOB read/write and index+1 overflow, but adds no regression tests for the new guards. `tests/test_array_api.c` already covers put_idx/get_idx extensively yet never exercises a negative index (get_idx should return NULL, put_idx should return -1), idx == INT_MAX (put_idx should return -1), or the expansion cap — so nothing in `make check` would catch a reversion of these fixes. Add CHK assertions for these cases in test_array_api.c (e.g. `fjson_object_array_get_idx(arr, -1) == NULL` and `fjson_object_array_put_idx(arr, -1, ...) == -1`).</comment>

<file context>
@@ -51,29 +53,42 @@ array_list_free(struct array_list *arr)
 {
+	/* reject a negative index (heap underflow) and an index whose +1 would
+	 * overflow int */
+	if(idx < 0 || idx == INT_MAX) return -1;
 	if(array_list_expand_internal(arr, idx+1)) return -1;
 	if(arr->array[idx]) arr->free_fn(arr->array[idx]);
</file context>

if(array_list_expand_internal(arr, idx+1)) return -1;
if(arr->array[idx]) arr->free_fn(arr->array[idx]);
arr->array[idx] = data;
Expand Down