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.

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

Potential Integer Overflow at function insertPitchPeriod #50

Open
spearo2 opened this issue May 1, 2024 · 0 comments
Open

Potential Integer Overflow at function insertPitchPeriod #50

spearo2 opened this issue May 1, 2024 · 0 comments

Comments

@spearo2
Copy link

spearo2 commented May 1, 2024

Dear authors,
There exists a potential integer overflow at the function insertPitchPeriod at

sonic/sonic.c

Line 1056 in 8694c59

if (!enlargeOutputBufferIfNeeded(stream, period + newSamples)) {

caused by period + newSamples which can lead to an allocation error at sonic.c:465:37 enlargeOutputBufferIfNeeded.

sonic/sonic.c

Lines 460 to 469 in 8694c59

static int enlargeOutputBufferIfNeeded(sonicStream stream, int numSamples) {
int outputBufferSize = stream->outputBufferSize;
if (stream->numOutputSamples + numSamples > outputBufferSize) {
stream->outputBufferSize += (outputBufferSize >> 1) + numSamples;
stream->outputBuffer = (short*)sonicRealloc(
stream->outputBuffer,
outputBufferSize,
stream->outputBufferSize,
sizeof(short) * stream->numChannels);

When the sum overflows, the argument numSamples becomes a negative value.
The allocation function potentially fails because the if guard at sonic.c:463 fails to filter the value of outputBufferSize.

A possible fix suggestion would be adding an additional safety function and using it before calling the function.
For example,

size_t sonicSafeAdd(size_t a, size_t b) {
    size_t sum = a + b;
    if (sum >= SIZE_MAX || sum < a) {
        /// handle exit
    }
    return sum;
}

Could be used as

- enlargeOutputBufferIfNeeded(stream, (newSamples + period); 
+ enlargeOutputBufferIfNeeded(stream, (sonicSafeAdd(newSamples, period));

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant