Potential NULL Pointer Dereference in pmFloatArrayNew and pmIntArrayNew

[mpap10]

We are researchers from the University of Athens, working on cross-language analysis of Python packages with C/C++ native extensions.

Problem

We found an issue in the Pymunk package: the functions pmFloatArrayNew() [1] and pmIntArrayNew() [2] do not check the return values of their internal memory allocation calls. After allocating memory for the outer struct using cpcalloc() [3],[4] each function immediately attempts to allocate the internal array (arr->arr) through a second cpcalloc() call [5], [6].
If this allocation fails, the internal buffer is NULL while the outer struct remains allocated, which can lead to NULL pointer dereference when the buffer is used.

Observed Behavior in Python

To explore the bug, we ran the following scripts for pmFloatArrayNew and pmIntArrayNew, respectively:
For pmFloatArrayNew()

from pymunk._chipmunk import lib as chiplib
SIZE = 250
COUNT = 10000000
y = []
for i in range(COUNT):
    p = chiplib.pmFloatArrayNew(SIZE)
    y.append(p)

For pmIntArrayNew()

from pymunk._chipmunk import lib as chiplib
SIZE = 250
COUNT = 10000000
y = []
for i in range(COUNT):
    p = chiplib.pmIntArrayNew(SIZE)
    y.append(p)

In practice, the interpreter raises a MemoryError in Python before cpcalloc() returns NULL, which means that a NULL pointer dereference is rarely observed directly as a segmentation fault. However, the underlying issue remains and can result in undefined behavior in memory-constrained scenarios or when very large allocations are requested.

Potential Fix

• Ensure that the internal allocation for arr->arr is checked before the function proceeds or returns the struct.
• Add defensive NULL checks so that code handling pmFloatArray or pmIntArray instances does not dereference a NULL internal buffer under any circumstances.

References

[1]

pymunk/pymunk_cffi/extensions.c

Line 72 in f3c3b38

pmFloatArrayNew(int size)

[2]

pymunk/pymunk_cffi/extensions.c

Line 134 in f3c3b38

pmIntArrayNew(int size)

[3]

pymunk/pymunk_cffi/extensions.c

Line 74 in f3c3b38

pmFloatArray *arr = (pmFloatArray *)cpcalloc(1, sizeof(pmFloatArray));

[4]

pymunk/pymunk_cffi/extensions.c

Line 136 in f3c3b38

pmIntArray *arr = (pmIntArray *)cpcalloc(1, sizeof(pmIntArray));

[5]

pymunk/pymunk_cffi/extensions.c

Line 78 in f3c3b38

arr->arr = (cpFloat *)cpcalloc(arr->max, sizeof(cpFloat));

[6]

pymunk/pymunk_cffi/extensions.c

Line 140 in f3c3b38

arr->arr = (uintptr_t *)cpcalloc(arr->max, sizeof(uintptr_t));

[viblo]

Thanks for the report!

It does however seem highly unlikely to cause any issues. This is not part of the public Pymunk API, and internally its only ever created with 0 size. I guess a more realistic case would be for realloc to fail, when the arrays grow..? Maybe that's good to fix? On the other hand, if it really did fail it seems like the way out is to exit the program anyway, I have a hard time to imagine a Pymunk user that handles OOM in a useful way...

On this topic I suspect there's more of this kinds of issues hidden. A sort of related issue that comes back from time to time is freeing memory when shutting down/GC. Always very annoying to troubleshoot and fix.