[PATCH] wprintf overflow

[Carmelo AMOROSO]

Kevin Cernekee wrote:
> Hi,
>
> I am seeing a buffer overflow in uClibc-0.9.28 on mipsel, illustrated by
> the following test program:
>
>
> #include <stdio.h>
> #include <wchar.h>
>
> int main(int argc, char **argv)
> {
> wprintf(L"This line is OK\n");
> wprintf(L"This line is no problem either because the format spec
> is at the end: %d\n", 1);
> wprintf(L"%d: This line will segfault because the format spec is
> too far from the end\n", 1);
> return(0);
> }
>
>
> In uClibc/libc/stdio/vfprintf.c or _vfprintf.c, function
> _ppfs_parsespec(), there is a loop that copies the const wchar_t format
> specifier into a char buffer so that it can be parsed using the same code.
> However, this loop terminates at the end of the string, rather than the
> end of the format spec, causing a buffer overflow for strings in which the
> format specifier begins more than 32 characters from the end of the
> string.
>
Hello, you analysis is correct...
> I don't see any evidence that this has been fixed in the SVN sources, but
> admittedly I have not tried building/running them either.
>
> My workaround is attached. A better solution might be to figure out when
> the format spec ends, and stop copying at that point.
yes, but I'd prefer to keep the code simpler as is, and copying the at 
maximum 32 bytes.
> This would allow us
> to reinstate the "char != wchar_t" check that errors out if the user puts
> a high character in the format spec. Another option would be to just
> store a NUL byte and quit copying if a high character is encountered,
> letting the format spec parser sort out any issues. Something like:
>
The fix I committed I think it's better... because solve the stack 
overflow but keep the check against
higher character.
I tested it and it works. Let me know your comments.

--- trunk/uClibc/libc/stdio/
_vfprintf.c 2008-02-07 00:53:36 UTC (rev 20951)
+++ trunk/uClibc/libc/stdio/_vfprintf.c 2008-02-07 07:06:49 UTC (rev 20952)
@@ -898,7 +898,7 @@
                               ) {
                               return -1;
                       }
-               } while (buf[i++]);
+               } while (buf[i++] && (i < sizeof(buf)));
               buf[sizeof(buf)-1] = 0;
       }
 #else  /* __UCLIBC_HAS_WCHAR__ */

Cheers,
Carmelo
>
> _______________________________________________
> uClibc mailing list
> uClibc at uclibc.org
> http://busybox.net/cgi-bin/mailman/listinfo/uclibc