sonic icon indicating copy to clipboard operation
sonic copied to clipboard

Potential Integer Overflow at function `insertPitchPeriod`

Open spearo2 opened this issue 10 months ago • 0 comments

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

https://github.com/waywardgeek/sonic/blob/8694c596378c24e340c09ff2cd47c065494233f1/sonic.c#L1056

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

https://github.com/waywardgeek/sonic/blob/8694c596378c24e340c09ff2cd47c065494233f1/sonic.c#L460-L469

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

spearo2 avatar May 01 '24 10:05 spearo2