| Summary: | Integer overflow/underflow in SDL_RegisterApp/SDL_UnregisterApp | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Coriiander <coriiander> |
| Component: | events | Assignee: | Sam Lantinga <slouken> |
| Status: | RESOLVED WONTFIX | QA Contact: | Sam Lantinga <slouken> |
| Severity: | normal | ||
| Priority: | P2 | ||
| Version: | 2.0.0 | ||
| Hardware: | All | ||
| OS: | Windows (All) | ||
You're absolutely correct, although the likelihood of it hitting INT_MAX is very low, and the consequences aren't significant. If you run into a real world case where this is an issue, please reopen this bug. |
There's a bug in SDL_RegisterApp and SDL_UnregisterApp in file SDL_windowsevents.c. In there some reference counting is going on to ensure the Win32 windowclass (or application class as you will) only gets registered once upon first class registration and only gets unregistered once upon last class unregistration. As there is no bounds checking being performed, the reference counter can overflow/underflow. If this happens you'll have a memory leak, because SDL_Appname will then not be freed in SDL_UnregisterApp. In SDL_RegisterApp: /* Only do this once... */ if (app_registered) { ++app_registered; return (0); } I'm aware that in normal situations this function shouldn't be called this often, but if it's called beyond INT_MAX times, then the reference counter will go below zero. Not only will this make unregistration of the window class/application class impossible in SDL_UnregisterApp, this will also lead to a memory leak since SDL_AppName will not be freed. While I'd consider a memory leak serious undesired, the failure of not unregistring the window class/application class is less problematic as the Win32 API will do this for you automatically on program shutdown. To fix this I suggest to do some bounds checking in both functions. Like so: if (app_registered) { if (INT_MAX > app_registered) { ++app_registered; } else { /* FAIL, Set error or something. It's a void function not being able to return success state */ return; } return (0); }