diff --git a/sonic.c b/sonic.c index 8e2f696..0223189 100644 --- a/sonic.c +++ b/sonic.c @@ -461,12 +461,23 @@ void sonicSetNumChannels(sonicStream stream, int numChannels) { allocateStreamBuffers(stream, stream->sampleRate, numChannels); } -/* Enlarge the output buffer if needed. */ -static int enlargeOutputBufferIfNeeded(sonicStream stream, int numSamples) { +/* Enlarge the output buffer if needed. Not static: exercised directly by + the white-box overflow tests in tests/overflow_test.c. */ +int enlargeOutputBufferIfNeeded(sonicStream stream, int numSamples) { int outputBufferSize = stream->outputBufferSize; + int growth; - if (stream->numOutputSamples + numSamples > outputBufferSize) { - stream->outputBufferSize += (outputBufferSize >> 1) + numSamples; + if (numSamples < 0) { + return 0; + } + /* Written as a subtraction, not "numOutputSamples + numSamples", so the + comparison itself can't signed-overflow when numSamples is huge. */ + if (stream->numOutputSamples > outputBufferSize - numSamples) { + growth = outputBufferSize >> 1; + if (numSamples > INT_MAX - outputBufferSize - growth) { + return 0; + } + stream->outputBufferSize = outputBufferSize + growth + numSamples; stream->outputBuffer = (short*)sonicRealloc( stream->outputBuffer, outputBufferSize, stream->outputBufferSize, sizeof(short) * stream->numChannels); @@ -1040,10 +1051,13 @@ static int skipPitchPeriod(sonicStream stream, short* samples, float speed, return newSamples; } -/* Insert a pitch period, and determine how much input to copy directly. */ -static int insertPitchPeriod(sonicStream stream, short* samples, float speed, - int period) { +/* Insert a pitch period, and determine how much input to copy directly. Not + static: exercised directly by the white-box overflow tests in + tests/overflow_test.c. */ +int insertPitchPeriod(sonicStream stream, short* samples, float speed, + int period) { long newSamples; + long totalSamples; short* out; int numChannels = stream->numChannels; @@ -1052,7 +1066,14 @@ static int insertPitchPeriod(sonicStream stream, short* samples, float speed, } else { newSamples = period; } - if (!enlargeOutputBufferIfNeeded(stream, period + newSamples)) { + /* period + newSamples is computed in long above, but + enlargeOutputBufferIfNeeded takes an int; reject here instead of + silently truncating a huge period into a small/negative int. */ + totalSamples = (long)period + newSamples; + if (totalSamples < 0 || totalSamples > INT_MAX) { + return 0; + } + if (!enlargeOutputBufferIfNeeded(stream, (int)totalSamples)) { return 0; } out = stream->outputBuffer + stream->numOutputSamples * numChannels; diff --git a/tests/Makefile b/tests/Makefile index 9597190..028128f 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -16,7 +16,8 @@ CFLAGS=-Wall -Wno-unused-function -g -ansi -fPIC -pthread -I .. #CFLAGS += -Wall -Wno-unused-function -ansi -fPIC -pthread -I .. TEST_SRC = \ -input_clamping_test.c +input_clamping_test.c \ +overflow_test.c CC=gcc diff --git a/tests/overflow_test.c b/tests/overflow_test.c new file mode 100644 index 0000000..f585cb8 --- /dev/null +++ b/tests/overflow_test.c @@ -0,0 +1,88 @@ +/* Sonic library + Copyright 2026 + Bill Cox + This file is part of the Sonic Library. + + This file is licensed under the Apache 2.0 license. +*/ + +#include "sonic.h" + +#include +#include + +#define SAMPLE_RATE 44100 +#define NUM_CHANNELS 1 + +/* enlargeOutputBufferIfNeeded and insertPitchPeriod are not declared static + in sonic.c specifically so these white-box tests can call them directly, + without going through the public streaming API to reach the overflow + paths. They are not part of the public sonic.h contract. */ +int enlargeOutputBufferIfNeeded(sonicStream stream, int numSamples); +int insertPitchPeriod(sonicStream stream, short* samples, float speed, + int period); + +int sonicTestEnlargeOutputBufferRejectsOverflow(void) { + sonicStream stream; + int result; + + stream = sonicCreateStream(SAMPLE_RATE, NUM_CHANNELS); + if (stream == NULL) { + fprintf(stderr, "sonicCreateStream failed\n"); + return 0; + } + /* A request this large can't be satisfied; the guard must fail cleanly + instead of signed-overflowing the internal buffer size arithmetic. */ + result = enlargeOutputBufferIfNeeded(stream, INT_MAX); + sonicDestroyStream(stream); + if (result != 0) { + fprintf(stderr, + "enlargeOutputBufferIfNeeded should reject an overflowing request\n"); + return 0; + } + return 1; +} + +int sonicTestEnlargeOutputBufferAcceptsNormalRequest(void) { + sonicStream stream; + int result; + + stream = sonicCreateStream(SAMPLE_RATE, NUM_CHANNELS); + if (stream == NULL) { + fprintf(stderr, "sonicCreateStream failed\n"); + return 0; + } + result = enlargeOutputBufferIfNeeded(stream, 128); + sonicDestroyStream(stream); + if (result != 1) { + fprintf(stderr, + "enlargeOutputBufferIfNeeded should accept a normal request\n"); + return 0; + } + return 1; +} + +int sonicTestInsertPitchPeriodRejectsOverflow(void) { + sonicStream stream; + short dummy[1]; + int result; + + stream = sonicCreateStream(SAMPLE_RATE, NUM_CHANNELS); + if (stream == NULL) { + fprintf(stderr, "sonicCreateStream failed\n"); + return 0; + } + dummy[0] = 0; + /* period this large makes period + newSamples overflow INT_MAX before + the fix, which used to truncate silently at the + enlargeOutputBufferIfNeeded call boundary. The function must return + failure before touching samples, so a 1-element buffer is safe here. */ + result = insertPitchPeriod(stream, dummy, 1.0f, INT_MAX); + sonicDestroyStream(stream); + if (result != 0) { + fprintf(stderr, + "insertPitchPeriod should reject an overflowing period\n"); + return 0; + } + return 1; +} diff --git a/tests/runtests.c b/tests/runtests.c index a7ef2e9..c87223d 100644 --- a/tests/runtests.c +++ b/tests/runtests.c @@ -16,6 +16,9 @@ int main(int argc, char** argv) { assert(sonicTestInputClamping()); assert(sonicTestInputsDontCrash()); + assert(sonicTestEnlargeOutputBufferRejectsOverflow()); + assert(sonicTestEnlargeOutputBufferAcceptsNormalRequest()); + assert(sonicTestInsertPitchPeriodRejectsOverflow()); printf("All tests passed.\n"); return 0; } diff --git a/tests/tests.h b/tests/tests.h index 9f08d06..f34da4a 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -12,6 +12,9 @@ extern "C" { int sonicTestInputClamping(void); int sonicTestInputsDontCrash(void); +int sonicTestEnlargeOutputBufferRejectsOverflow(void); +int sonicTestEnlargeOutputBufferAcceptsNormalRequest(void); +int sonicTestInsertPitchPeriodRejectsOverflow(void); #ifdef __cplusplus }