We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 3668

Summary: Overflow of SDL_AudioCVT.filters with some downmixes
Product: SDL Reporter: Simon Hug <chli.hug>
Component: audioAssignee: Ryan C. Gordon <icculus>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P2    
Version: HG 2.0   
Hardware: All   
OS: All   
Attachments: Patch that fixes overflow of filters array in SDL_AudioCVT that happens with some downmixes.

Description Simon Hug 2017-06-11 00:20:20 UTC 
Created attachment 2760 [details]
Patch that fixes overflow of filters array in SDL_AudioCVT that happens with some downmixes.

There's a chance that an audio conversion from many channels to a few can use more than 9 audio filters. SDL_AudioCVT has 10 SDL_AudioFilter pointers of which one has to be the terminating NULL pointer. The SDL code has no checks for this limit. If it overflows there can be stack or heap corruption or a call to 0xa.

Attached patch adds a function that checks for this limit and throws an error if it is reached. Also adds some documentation.

Test parameters that trigger this issue:
AUDIO_U16MSB with 224 channels at 46359 Hz
                 V
AUDIO_S16MSB with 6 channels at 27463 Hz

The fuzzer program I uploaded in bug 3667 has more of them.
Comment 1 Sam Lantinga 2017-06-12 23:39:29 UTC 
Fixed, thanks!
https://hg.libsdl.org/SDL/rev/819384789a7a
Comment 2 Ryan C. Gordon 2017-06-13 01:14:11 UTC 
Technically, we should probably refuse to convert 224 channels at all, though.
Comment 3 Ryan C. Gordon 2017-06-13 01:37:47 UTC 
(In reply to Ryan C. Gordon from comment #2)
> Technically, we should probably refuse to convert 224 channels at all,
> though.

...and now we do: https://hg.libsdl.org/SDL/rev/62a0a6e9b48b

(the patch that Sam pushed is also a good fix for several other reasons, though, so this doesn't replace it.)

--ryan.