Skip to content
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

Jump to bottom

Fix for buffer overflow in OnOverlapCreatedTask #7

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

Fix for buffer overflow in OnOverlapCreatedTask #7

wants to merge 1 commit into from

Commits on Jan 26, 2017

  1. Fix for buffer overflow in OnOverlapCreatedTask 

    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.
    henryfalconer authored Jan 26, 2017
    Configuration menu
    Copy the full SHA
    a8decae View commit details
    Browse the repository at this point in the history