| Summary: | Out-of-bound access by SDL_ResampleAudioSimple on input buffer with certain parameters | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Simon Hug <chli.hug> |
| Component: | audio | Assignee: | Ryan C. Gordon <icculus> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | blocker | ||
| Priority: | P2 | CC: | sfalexrog |
| Version: | HG 2.0 | Keywords: | target-2.0.6 |
| Hardware: | All | ||
| OS: | All | ||
| Attachments: | AudioCVT Fuzzer 0.1 | ||
Created attachment 2759 [details]
AudioCVT Fuzzer 0.1
Attaching a fuzzer for the audio conversion functions. The code is not very clean, sorry. Hope it's going to make testing easier in some form or another.
There are known issues in it that will trigger the issues mentioned here. See option -k.
Hopefully we'll be replacing the resampler in the next few days (again!), so let's leave this open for now. --ryan. (Sorry if you get a lot of copies of this email, we're touching dozens of bug reports right now.) Tagging a bunch of bugs as target-2.0.6. This means we're in the final stretch for an official SDL 2.0.6 release! These are the bugs we really want to fix before shipping if humanly possible. That being said, we don't promise to fix them because of this tag, we just want to make sure we don't forget to deal with them before we bless a final 2.0.6 release, and generally be organized about what we're aiming to ship. After some debate, we might just remove this tag again and deal with it for a later release. Hopefully you'll hear more about this bug soon. If you have more information (including "this got fixed at some point, nevermind"), we would love to have you come add more information to the bug report when you have a moment. Thanks! --ryan. While trying to figure out why sound in one of my ports is buggy with SDL 2.0.6, I've stumbled upon this bug. Valgrind kept showing me accesses before the allocated array. Replacing all instances of const int pos = ((int) idx) * chans; with const int pos = ((int) idx + 1) * chans; seemed to prevent resamplers from accessing data out-of-bounds. Not sure if it is a correct fix, though. Ok, first off, that Fuzzer program is _awesome_. :) I've thrown out the current resampler for something with some actual research backing the math involved: https://hg.libsdl.org/SDL/rev/a8382e3d0b54 The new one doesn't trigger any errors with the Fuzzer and/or Valgrind (minus some unrelated assertions the channel conversions trigger elsewhere). --ryan. |
It's possible for SDL_ResampleAudioSimple to read out of bounds of inbuflen when SDL_ConvertAudio is called with certain parameters. I'm not sure what exactly the cause is. At first I thought this has to do with the assertion at SDL_audiocvt.c:250. It asserts that the input length of the data (inbuflen) is a multiple of the framesize. This isn't always the case. However, it also happens with this simple example where this assertions holds: SDL_Init(SDL_INIT_AUDIO); SDL_AudioCVT cvt; if (SDL_BuildAudioCVT(&cvt, AUDIO_F32MSB, 1, 14273, AUDIO_S16LSB, 1, 45382) == 1) { Uint8 buf[1024]; cvt.buf = buf; cvt.len = 1024 / cvt.len_mult; SDL_ConvertAudio(&cvt); } It's always on one of the 'const float val = *(--src);' lines. It eventually gets to a position where it tries to read inbuf[-1]. There's another issue that has a similar segfault. If cvt->len is below the framesize, the divison at SDL_ResampleAudioSimple:241 will result in 0, causing another out-of-bound access. I think this might be fixable by enforcing some rule in SDL_ConvertAudio that cvt->len has to be positive and a multiple of the framesize.