NVIDIAGameWorks / PhysX-3.4

NVIDIA PhysX SDK 3.4
https://www.nvidia.com/
2.35k stars 274 forks source link

Fix for buffer overflow in OnOverlapCreatedTask #7

Open henryfalconer opened 7 years ago

henryfalconer commented 7 years ago

After a crash occurred in our game, I found out that OnOverlapCreatedTask::runInternal could read off the end of an allocated buffer.

The local variables currentSI and currentEI are pointers into the mShapeInteractions and mInteractionMarkers buffers respectively. Whenever an item in the buffer is used by an interaction, the pointer into the buffer (currentSI or currentEI) is incremented. Once all the items in the buffer have been used, the pointer will point to the memory address directly after the buffer, so if any code dereferences the pointer, it will be reading memory out of bounds, which rarely causes problems in practice but can occasionally cause a crash.

The problem is that the call to mNPhaseCore->createRbElementInteraction on line 6232 dereferences both currentSI and currentEI, so once one of those pointers reaches the end of its buffer, this line will perform an invalid read.

My change makes the affected buffers one element larger than they need to be, so that OnOverlapCreatedTask doesn't read invalid memory. This seemed better for performance than adding a check to ensure that the memory reads are within the bounds of the buffer, and the memory overhead is negligible.

I previously had a 100% repro for a crash caused by this bug, and after making this change the crash no longer occurs.

MikeRuete commented 6 years ago

Just because the memory is allocated doesn't mean its contents are valid. Based on your description, it doesn't sound like a good fix.