Potential Integer Overflow in cpSpaceAddCachedArbiter

[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 function cpSpaceAddCachedArbiter() [1] does not check arb->count [2] before copying them.
If arb->count is large, multiplying it by the size of the contact structure [3] can overflow, producing a smaller value. This causes memcpy() [3] to write past the allocated memory, which can lead to crashes or segmentation faults.

Steps to Reproduce

Open a terminal and run the following Python script:

import pymunk, sys

ffi = pymunk._chipmunk.ffi
lib = pymunk._chipmunk.lib

space = pymunk.Space()
arb = ffi.new("cpArbiter*")
arb.contacts = ffi.new("struct cpContact[]", 1)

count = 1000
for _ in range(10):
    print(count); sys.stdout.flush()
    arb.count = count
    lib.cpSpaceAddCachedArbiter(space._space, arb)
    count *= 2

You should observe an immediate crash:

1000
Segmentation fault (core dumped)

Potential Fix

• Validate the contact count to prevent an integer overflow when multiplying by the size of the contact structure.
• Ensure the total size does not overflow and fits within the allocated buffer before memcpy call.

References

[1]

pymunk/pymunk_cffi/extensions.c

Line 404 in f3c3b38

void cpSpaceAddCachedArbiter(cpSpace *space, cpArbiter *arb)

[2]

pymunk/pymunk_cffi/extensions.c

Line 409 in f3c3b38

int numContacts = arb->count;

[3]

pymunk/pymunk_cffi/extensions.c

Line 414 in f3c3b38

memcpy(arb->contacts, contacts, numContacts * sizeof(struct cpContact));

[viblo]

Thanks for the report!

When I run you repro-code I do get a crash, but not a segfault.

Aborting due to Chipmunk error: 
Internal Error: Contact buffer overflow!
	Failed condition: count <= CP_MAX_CONTACTS_PER_ARBITER
	Source: Munk2D/src/cpSpaceStep.c:177

Which is expected since there cant be more than 2 contacts for one arbiter. I guess it could segfault if you are very low on memory? On the other hand, this is an internal function in Pymunk, only used when unpickling a Space, so it would have to be quite extreme for it to fail.
(I havent really had the time to think about this and the other issue you opened)

Can I ask, how do analyze the code? Some automated way I assume?

(Sorry for somewhat late reply, I have been away abroad during the holidays).