Effect-chain processing can write past the original buffer when non-in-place effects expand channel count #404

New issue

Open

opened 2026-05-17 10:45:29 -05:00 by sbvoxel · 2 comments

sbvoxel commented

2026-05-17 10:45:29 -05:00

(Migrated from github.com)

FAudio already has a FIXME in FAudio_INTERNAL_ProcessEffectChain for this case:

/* FIXME: What if this is smaller because
* inputChannels < desc[i].OutputChannels?
*/
dstParams.pBuffer = buffer;

I confirmed this as a real out-of-bounds write with a regression test in my hard fork:

github.com/sbvoxel/ForgeAudio@cb2532e9d5

The failing shape is:

The original input buffer has C channels.
Effect 0 is non-in-place and expands C -> E.
Effect 1 is non-in-place and outputs E -> E or larger.
The second effect’s destination becomes the original buffer.
The zero-fill before the second effect callback writes E * frame_count floats into storage that may only be sized for C * frame_count.

So the overflow happens before the second effect’s Process call.

In my fork, I fixed this by adding a second reusable scratch buffer and making non-in-place effects ping-pong between scratch buffers instead of writing expanded output back into the original input buffer.

The fix is probably not directly PR-ready because my fork has renamed APIs/types, but the regression test and the buffer-flow issue should be easy to inspect. This was AI-assisted work, so please treat the linked commit as a starting point for review rather than a drop-in upstream patch.

FAudio already has a FIXME in `FAudio_INTERNAL_ProcessEffectChain` for this case: ```c /* FIXME: What if this is smaller because * inputChannels < desc[i].OutputChannels? */ dstParams.pBuffer = buffer; ``` I confirmed this as a real out-of-bounds write with a regression test in my hard fork: https://github.com/sbvoxel/ForgeAudio/commit/cb2532e9d5e6c5fc80777f89169341431dc1bfda The failing shape is: 1. The original input buffer has C channels. 2. Effect 0 is non-in-place and expands C -> E. 3. Effect 1 is non-in-place and outputs E -> E or larger. 4. The second effect’s destination becomes the original buffer. 5. The zero-fill before the second effect callback writes E * frame_count floats into storage that may only be sized for C * frame_count. So the overflow happens before the second effect’s Process call. In my fork, I fixed this by adding a second reusable scratch buffer and making non-in-place effects ping-pong between scratch buffers instead of writing expanded output back into the original input buffer. The fix is probably not directly PR-ready because my fork has renamed APIs/types, but the regression test and the buffer-flow issue should be easy to inspect. This was AI-assisted work, so please treat the linked commit as a starting point for review rather than a drop-in upstream patch.

sbvoxel commented

2026-05-18 09:52:50 -05:00

(Migrated from github.com)

You may also be interested in these:

github.com/sbvoxel/ForgeAudio@13dcaf33c6
github.com/sbvoxel/ForgeAudio@4b55c6ead6

Again, AI work.

I'm only highlighting what seems most important.

You may also be interested in these: https://github.com/sbvoxel/ForgeAudio/commit/13dcaf33c685ad5169f0f2cb04b0da002cce403e https://github.com/sbvoxel/ForgeAudio/commit/4b55c6ead6931ff9f8c13819deabf009a55311a4 Again, AI work. I'm only highlighting what seems most important.

sbvoxel commented

2026-05-19 06:55:53 -05:00

(Migrated from github.com)

Two of the newest AI models think we fixed a wrong cone direction: github.com/sbvoxel/ForgeAudio@b0dcba6380

Out of bounds issue fixed: github.com/sbvoxel/ForgeAudio@3c317a64f6

I'm not going to create an issue per finding, because again this is AI work. But I do find it a good idea to leave a thread here for the largest findings if someone wants to take something from this for FAudio.

Two of the newest AI models think we fixed a wrong cone direction: https://github.com/sbvoxel/ForgeAudio/commit/b0dcba6380fa7f2e8b24a5e88c8dd131c460db71 Out of bounds issue fixed: https://github.com/sbvoxel/ForgeAudio/commit/3c317a64f65595e7d748e35717e9d6759457d755 I'm not going to create an issue per finding, because again this is AI work. But I do find it a good idea to leave a thread here for the largest findings if someone wants to take something from this for FAudio.