We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 2049 - SDL_GetErrorMsg infinite loop
Summary: SDL_GetErrorMsg infinite loop
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: *don't know* (show other bugs)
Version: 2.0.0
Hardware: x86 Linux
: P2 normal
Assignee: Ryan C. Gordon
QA Contact: Sam Lantinga
URL:
Keywords: triage-2.0.4
Depends on:
Blocks:
 
Reported: 2013-08-21 13:24 UTC by Donny
Modified: 2015-03-24 07:15 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Donny 2013-08-21 13:24:47 UTC
I'm hitting an infinite loop in SDL_GetErrorMsg in SDL_error.c

Happens in while loop @ line 141 (SDL2-2.0.0 release tarball)
Comment 1 Sam Lantinga 2013-09-07 06:47:45 UTC
What's the error message that it's trying to parse?

Can you print out the entire error structure?

Thanks!
Comment 2 Ryan C. Gordon 2014-02-23 05:49:13 UTC
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.
Comment 3 Ryan C. Gordon 2014-02-23 05:59:39 UTC
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.
Comment 4 Ryan C. Gordon 2015-02-19 05:22:21 UTC
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!
Comment 5 Ryan C. Gordon 2015-03-24 06:50:23 UTC
Going to fix those concerns and close this bug.

--ryan.
Comment 6 Ryan C. Gordon 2015-03-24 06:57:45 UTC
(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.
Comment 7 Ryan C. Gordon 2015-03-24 07:15:19 UTC
(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.