arraylist: reject negative indices and size overflow

[prownd]

• array_list_get_idx() did not reject a negative index, so a negative argument read before the start of the backing array.
• array_list_put_idx() passed idx unchecked to array_list_expand_internal() as idx + 1, and the expansion did new_size = arr->size << 1 and new_size * sizeof(void*). A negative index wrote out of bounds and a very large one overflowed the signed arithmetic (both flagged by ASan/UBSan).

Reject a negative or INT_MAX index in put_idx and a negative index in get_idx, and cap the growth so neither the doubling nor the byte-size computation can overflow.

---

Summary by cubic

Hardens array list indexing and capacity growth against invalid indices and integer overflow. Negative get_idx calls now return NULL instead of reading before the backing array; put_idx rejects negative and INT_MAX indices, and oversized growth fails without overflowing.

^{Written for commit 7d358f2. Summary will update on new commits.}

[cubic-dev-ai]

1 issue found across 1 file

Confidence score: 5/5

• arraylist.c fixes the sanitizer-identified OOB access and index+1 overflow, but the guards lack regression coverage; extend tests/test_array_api.c to exercise negative and boundary indices through put_idx/get_idx.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="arraylist.c">

<violation number="1" location="arraylist.c:91">
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`).</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

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>