set_error can violate memory safety

[LunarLambda]

The set_error function turns the given str to a CString and passes it directly to SDL_SetError.

Note that SDL_SetError is a variadic function, and accepts a printf-like format string.

The problem with this is that the number of arguments is derived from the format string, and in most ABIs, variadic functions take their parameters on the stack

In effect, this means that code such as this:

SDL_SetError("%p%p%p%p%p%p%p%p");

or the rust equivalent

sdl2::set_error("%p%p%p%p%p%p%p%p");

will likely cause a crash due to corrupting the stack.

EDIT: This particular example does not crash on my machine, although it does allow to read memory from the stack, including the function's return address.

However, using a different specifier like %s will almost definitely cause a segmentation fault.
Furthermore, SDL_vsnprintf, which handles formatting for SDL_SetError, will delegate to the native C library's vsnprintf function when possible, meaning the %n specifier can be used to write to arbitrary (and likely invalid) memory, which definitely should not be possible from within safe rust.

Potential Fixes

1. Remove set_error, as it is difficult to use correctly
2. Make set_error unsafe, and document why
3. Make set_error check for format specifiers, and fail if any are found
4. Change set_error such that it passes "%s" as the format string, and the passed str as the only other argument

Documentation for SDL_SetError
Source code for SDL_vsnprintf
Article about format string attacks

[Cobrand]

4. is the wisest, in C with printf you always use %s instead of custom strings as the first argument because of this reason (in the rare cases that you do). Users that will want a variadic function can always use sdl2_sys::SDL_SetError