| Summary: | SDL_GetErrorMsg infinite loop | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Donny <donny.springer> |
| Component: | *don't know* | Assignee: | Ryan C. Gordon <icculus> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | normal | ||
| Priority: | P2 | CC: | icculus |
| Version: | 2.0.0 | Keywords: | triage-2.0.4 |
| Hardware: | x86 | ||
| OS: | Linux | ||
|
Description
Donny
2013-08-21 13:24:47 UTC
What's the error message that it's trying to parse? Can you print out the entire error structure? Thanks! I can't see a way this loop would be infinite, but you can definitely jump off the end of the format string if it ends with a '%' char. The "switch (spot[-2])" should have a "case '\0'" section that bails out in this case. --ryan. And I guess if SDL_snprintf() returns a negative number, which it does with some C runtimes, we have issues, too. Maybe in a pathological case, we write before the start of a stack array and it replaces fmt with a valid pointer that keeps pushing us into a loop that writes over fmt again? On other C runtimes, SDL_snprintf() returns characters it _would_ have written if there were space, which means we'll probably overflow the other end of the array on those systems. In either case, we check if maxlen > 0, but it's unsigned, so either bug will probably cause it to grow or wrap around to 0xFFFFFFFF. It's the best I've got. These are real bugs in any case, even if we can't to the infinite loop. --ryan. Marking a large number of bugs with the "triage-2.0.4" keyword at once. Sorry if you got a lot of email from this. This is to help me sort through some bugs in regards to a 2.0.4 release. We may or may not fix this bug for 2.0.4, though! Going to fix those concerns and close this bug. --ryan. (In reply to Ryan C. Gordon from comment #2) > I can't see a way this loop would be infinite, but you can definitely jump > off the end of the format string if it ends with a '%' char. > > The "switch (spot[-2])" should have a "case '\0'" section that bails out in > this case. This one can't happen, I think; SDL_SetError() checks for this case, and it looks like maybe this will happen to push past this bogus char elsewhere in the loop anyhow. --ryan. (In reply to Ryan C. Gordon from comment #3) > In either case, we check if maxlen > 0, but it's unsigned, so either bug > will probably cause it to grow or wrap around to 0xFFFFFFFF. Made maxlen signed, made sure we ignore snprintf() results unless they're > 0, make sure if snprintf() reports a number larger than the amount it wrote that we don't overflow our output buffer. https://hg.libsdl.org/SDL/rev/92622f92bb8c Marking this FIXED. Please reopen if this bug still presents for you, but I suspect this will take care of the problem. --ryan. |