Fixed bug 3531 - internal SDL_vsnprintf implementation access memory outside given buffer ranges
authorSam Lantinga <slouken@libsdl.org>
Sat, 31 Dec 2016 16:14:51 -0800
changeset 107335f8d2376d5b2
parent 10732 617f3de85b0a
child 10734 a4bf6eab5aef
Fixed bug 3531 - internal SDL_vsnprintf implementation access memory outside given buffer ranges

Tristan

The internal SDL_vsnprintf implementation accesses memory outside buffer. The bug existed also inside the format (%) processing, which was fixed with Bug 3441.

But there is still an invalid access, if we do not have any format inside the source string and the destination string is shorter than the format string. You can use any string for this test, as long it is longer than the buffer.

Example:

va_list argList;
char buffer[4];
SDL_vsnprintf(buffer, sizeof(buffer), "Testing", argList);

The bug is located on the 'else' branch of the format char test:

while (*fmt) {
if (*fmt == '%') {
...
} else {
if (left > 1) {
*text = *fmt;
--left;
}
++fmt;
++text;
}
}
if (left > 0) {
*text = '\0';
}

As you can see that text is always incremented, even when left is already one. When then on the last lines, *text is assigned the NULL char, the pointer is located outside bounds.
src/stdlib/SDL_string.c
     1.1 --- a/src/stdlib/SDL_string.c	Sat Dec 31 10:30:07 2016 -0800
     1.2 +++ b/src/stdlib/SDL_string.c	Sat Dec 31 16:14:51 2016 -0800
     1.3 @@ -1484,7 +1484,7 @@
     1.4      if (!fmt) {
     1.5          fmt = "";
     1.6      }
     1.7 -    while (*fmt) {
     1.8 +    while (*fmt && left > 1) {
     1.9          if (*fmt == '%') {
    1.10              SDL_bool done = SDL_FALSE;
    1.11              size_t len = 0;
    1.12 @@ -1646,12 +1646,8 @@
    1.13                  left -= len;
    1.14              }
    1.15          } else {
    1.16 -            if (left > 1) {
    1.17 -                *text = *fmt;
    1.18 -                --left;
    1.19 -            }
    1.20 -            ++fmt;
    1.21 -            ++text;
    1.22 +            *text++ = *fmt++;
    1.23 +            --left;
    1.24          }
    1.25      }
    1.26      if (left > 0) {