| Summary: | crash in vfprintf with more than 64 format args and format specifiers (CVE-2012-3405) | ||
|---|---|---|---|
| Product: | glibc | Reporter: | Rich Coe <rcoe> |
| Component: | libc | Assignee: | Ulrich Drepper <drepper.fsp> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | aj, fweimer, gnu, vapier |
| Priority: | P2 | Flags: | fweimer:
security+
|
| Version: | 2.14 | ||
| Target Milestone: | --- | ||
| URL: | https://bugzilla.novell.com/show_bug.cgi?id=733140 | ||
| Host: | Target: | ||
| Build: | Last reconfirmed: | ||
| Attachments: |
testcase
patch |
||
Created attachment 6078 [details]
patch
# patch to fix printf when register_printf_function is in use and the format to
# printf contains more than 64 elements.
#
# extend_alloca modifies nspecs_max (2nd arg) with the size of the space, not the
# number of elements. Use a temporary to store the return value. Explicitly set the correct size.
Thanks, this patch fixes indeed the testcase. The following comment was made to the downstream bugreport https://bugzilla.novell.com/show_bug.cgi?id=733140#c6 by Christoph Bartoschek: I would suggest the following patch instead. This way nsize is initialized with the correct number of bytes. And nspecs_max uses the whole given buffer. diff --git a/stdio-common/vfprintf.c b/stdio-common/vfprintf.c index 753a5ac..6e026ae 100644 --- a/stdio-common/vfprintf.c +++ b/stdio-common/vfprintf.c @@ -1683,8 +1683,9 @@ do_positional: { /* Extend the array of format specifiers. */ struct printf_spec *old = specs; - specs = extend_alloca (specs, nspecs_max, - 2 * nspecs_max * sizeof (*specs)); + size_t nsize = nspecs_max * sizeof(*specs); + specs = extend_alloca (specs, nsize, 2 * nsize); + nspecs_max = nsize/sizeof(*specs); /* Copy the old array's elements to the new space. */ memmove (specs, old, nspecs * sizeof (struct printf_spec)); nsize does not always represent the correct size of the returned object, due to the way that extend_alloca (an internal glibc macro) is implemented. I wouldn't recommend this latest proposal. I suggest that the patch from #3 is used. - extend_alloca sets the second parameter to a value that can safely be used. There is no possibility that nsize is larger than the usable buffer size after extend_alloca. - If one does not want to use the extending feature of extend_alloca then one should just use alloca. I checked in a correct patch. The patch is wrong, you need to increase nspecs_size in the for loop if you do a new alloca call. Otherwise you call alloca every time with nspecs_size*2 - and eventually override the array Add after: specs = extend_alloca (specs, nspecs_size, 2 * nspecs_size); a line with: nspecs_size = 2 * nspecs_size; That's what extend_alloca does. *** Bug 260998 has been marked as a duplicate of this bug. *** Seen from the domain http://volichat.com Page where seen: http://volichat.com/adult-chat-rooms Marked for reference. Resolved as fixed @bugzilla. for reference, the change: https://sourceware.org/git/?p=glibc.git;a=commit;h=a4647e727a2a52e1259474c13f4b13288938bed4 |
Created attachment 6077 [details] testcase A user found this in an application that uses format specifiers and motif was initializing. Program received signal SIGSEGV, Segmentation fault. 0x0000000000000000 in ?? () (gdb) bt #0 0x0000000000000000 in ?? () #1 0x00007ffff7a91ec8 in _IO_vfprintf_internal (s=<optimized out>, format=<optimized out>, ap=<optimized out>) at vfprintf.c:1739 #2 0x00007ffff7ab5024 in __IO_vsprintf (string=0x603020 "", format=0x601080 "%%P%%S:%s/%%L/%%T/%%N/%%P%%S:%s/%%l_%%t/%%T/%%N/%%P%%S:%s/%%l/%%T/%%N/%%P%%S:%s/%%T/%%N/%%P%%S:%s/%%L/%%T/%%P%%S:%s/%%l_%%t/%%T/%%P%%S:%s/%%l/%%T/%%P%%S:%s/%%T/%%P%%S:%s/%%P%%S:%s/%%L/%%T/%%N/%%P%%S:%"..., args=0x7fffffffc6c8) at iovsprintf.c:43 #3 0x00007ffff7a9c0a7 in __sprintf (s=<optimized out>, format=<optimized out>) at sprintf.c:34 #4 0x00000000004009d3 in main (argc=1, argv=0x7fffffffd928) at mtest.c:98