| Summary: | Checks in SDL_JoystickGetXXX functions for valid ranges are wrong | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Jools Wills <buzz> |
| Component: | joystick | Assignee: | Sam Lantinga <slouken> |
| Status: | RESOLVED INVALID | QA Contact: | Sam Lantinga <slouken> |
| Severity: | normal | ||
| Priority: | P2 | ||
| Version: | HG 2.1 | ||
| Hardware: | x86_64 | ||
| OS: | Linux | ||
| Attachments: | Adjust check in Joystick functions when checking against number of Buttons/Axes/Hats/Balls | ||
If I am right then I'm not suggesting this diff is used as is. The error messages would be better changed also and the +1 might be better on the right side of the logic as a -1 (or just implemented differently), but if it's checking for out of bounds buttons etc, it should handle there being 0 buttons/axes etc. I meant btw
if ((axis + 1 ) < joystick->naxes) {
or if (axis < ( joystick->naxes - 1)) {
Sorry - I'm obviously overtired. There is an issue with a crash but this isn't the problem. <y logic is all wrong - sorry. If button 0 is requested and joystick->nbuttons is 0, then it wouldn't pass the check. The issue is the application I was debugging passing a button code of -1 in. Maybe SDL could do a check for this though.and return an error instead of segfaulting (just needs an additional check that button/axes are >= 0) I will open a new issue once I've fixed the app. Apologies again for my brain glitch with the initial report. Would a patch to SDL to throw an error if a button/axis/hat/ball that was < 0 be accepted? Maybe preferable over a segfault even though the app is at fault here. Sure, that sounds good. Thanks! |
Created attachment 4094 [details] Adjust check in Joystick functions when checking against number of Buttons/Axes/Hats/Balls Unless I'm missing something the range checks in the SDL_JoystickGetAxis / GetButton / GetHat / GetBall are all incorrect. Whilst debugging some code that crashed with a Wii controller connected (That has some Joystick devices that lack Axes / Buttons), I decided to debug the crash which was in SDL. The problem in the code was it was doing calls to SDL_JoystickGetButton without checking SDL_JoystickNumButtons first. It seemed logical though that SDL shouldn't segfault and should return a failure. I checked the functions and they all seem incorrect. SDL_JoystickGetAxis has if (axis < joystick->naxes) If joystick->naxes is 0 then asking for Axis 0 will pass this check. It would be logical for it to be if ((axis + 1 ) < joystick->naxes) { or if (axis < ( joystick->naxes + 1)) { same for the other functions. See attached diff which resolves the problem. I am wondering if I'm missing something here though as I would have thought this would have been picked up before - so I apologise if I have missed something obvious. Checking the Docs all the Button / Axes / Hat / Ball indices start at 0.