Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SDL_LoadWAV() crashes with invalid files #310

Closed
zmanuel opened this issue Aug 6, 2023 · 0 comments · Fixed by #311
Closed

SDL_LoadWAV() crashes with invalid files #310

zmanuel opened this issue Aug 6, 2023 · 0 comments · Fixed by #311

Comments

@zmanuel
Copy link
Contributor

zmanuel commented Aug 6, 2023

With native SDL 1.2, that used to return NULL and set an appropriate error.

To reproduce, simply call the loopwave test with a nonsense argument.

The fix is a simple standard early NULL check with return in SDL_LoadWAV_RW, I'll submit a PR momentarily.

We found this because our silly old game uses SDL_LoadWAV to just speculatively load optional sound files, and that crashes now: https://forums3.armagetronad.net/viewtopic.php?t=39657
A workaround is already in the release pipeline, so no hurry.

Looking around:
The similar SDL_LoadBMP(_RW) handles NULL arguments fine already.
I have not looked too deeply into SDL_SaveBMP. Calling it with both arguments NULL crashes, too, but because of the NULL surface argument; in native SDL1.2, a NULL surface and a valid file would already crash.

zmanuel added a commit to zmanuel/sdl12-compat that referenced this issue Aug 6, 2023
Native SDL 1.2 and 2.0 both handle SDL_LoadWAV(null,...), SDL_LoadWAV("",...) and SDL_LoadWAV(nonexistent_file,...)
by returning NULL and setting an error; this check resores that
behavior.

Fixes libsdl-org#310
icculus pushed a commit that referenced this issue Aug 6, 2023
Native SDL 1.2 and 2.0 both handle SDL_LoadWAV(null,...), SDL_LoadWAV("",...) and SDL_LoadWAV(nonexistent_file,...)
by returning NULL and setting an error; this check resores that
behavior.

Fixes #310
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant