Bug 13446 (CVE-2012-3405)

Summary: crash in vfprintf with more than 64 format args and format specifiers (CVE-2012-3405)
Product: glibc Reporter: Rich Coe <rcoe>
Component: libcAssignee: Ulrich Drepper <drepper.fsp>
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

Description Rich Coe 2011-11-29 03:37:22 UTC
Created attachment 6077 [details]

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
Comment 1 Rich Coe 2011-11-29 03:39:22 UTC
Created attachment 6078 [details]

# 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.
Comment 2 Andreas Jaeger 2011-11-29 08:55:50 UTC
Thanks, this patch fixes indeed the testcase.
Comment 3 Andreas Jaeger 2011-11-29 10:38:41 UTC
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));
Comment 4 Rich Coe 2011-11-29 12:22:40 UTC
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.
Comment 5 Christoph Bartoschek 2011-12-05 12:43:54 UTC
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.
Comment 6 Ulrich Drepper 2011-12-18 02:29:12 UTC
I checked in a correct patch.
Comment 7 Andreas Jaeger 2011-12-18 06:32:26 UTC
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;
Comment 8 Andreas Schwab 2011-12-18 09:10:10 UTC
That's what extend_alloca does.
Comment 9 Jackie Rosen 2014-02-16 19:42:16 UTC Comment hidden (spam)
Comment 10 Mike Frysinger 2014-06-14 22:35:54 UTC
for reference, the change: