[src/trunk]: src/lib/libc/stdio Fix: CVE-2012-0864 fprintf() positional argum...

[christos <christos%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/da17397e05d0
branches:  trunk
changeset: 777413:da17397e05d0
user:      christos <christos%NetBSD.org@localhost>
date:      Fri Feb 17 19:57:53 2012 +0000

description:
Fix: CVE-2012-0864 fprintf() positional argument abuse.
Described in: http://www.phrack.org/issues.html?issue=67&id=9#article
Reported by Stefan Cornelius / Red Hat Security Response Team

- convert internal positional arguments bookkeeping from int to size_t
- provide overflow protection in positional argument spec
- convert loops to memset
- fix memory leaks
- limit positional argument stack offset to the number of arguments required
  by the printf to avoid coredump from va_arg() exhaustion.

diffstat:

 lib/libc/stdio/vfwprintf.c |  68 +++++++++++++++++++++++++++++++--------------
 1 files changed, 46 insertions(+), 22 deletions(-)

diffs (178 lines):

diff -r 320b4ae8c3f7 -r da17397e05d0 lib/libc/stdio/vfwprintf.c
--- a/lib/libc/stdio/vfwprintf.c        Fri Feb 17 19:00:45 2012 +0000
+++ b/lib/libc/stdio/vfwprintf.c        Fri Feb 17 19:57:53 2012 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: vfwprintf.c,v 1.24 2011/08/17 09:53:54 christos Exp $  */
+/*     $NetBSD: vfwprintf.c,v 1.25 2012/02/17 19:57:53 christos Exp $  */
 
 /*-
  * Copyright (c) 1990, 1993
@@ -38,7 +38,7 @@
 static char sccsid[] = "@(#)vfprintf.c 8.1 (Berkeley) 6/4/93";
 __FBSDID("$FreeBSD: src/lib/libc/stdio/vfwprintf.c,v 1.27 2007/01/09 00:28:08 imp Exp $");
 #else
-__RCSID("$NetBSD: vfwprintf.c,v 1.24 2011/08/17 09:53:54 christos Exp $");
+__RCSID("$NetBSD: vfwprintf.c,v 1.25 2012/02/17 19:57:53 christos Exp $");
 #endif
 #endif /* LIBC_SCCS and not lint */
 
@@ -124,7 +124,7 @@
  * Type ids for argument type table.
  */
 enum typeid {
-       T_UNUSED, TP_SHORT, T_INT, T_U_INT, TP_INT,
+       T_UNUSED = 0, TP_SHORT, T_INT, T_U_INT, TP_INT,
        T_LONG, T_U_LONG, TP_LONG, T_LLONG, T_U_LLONG, TP_LLONG,
        T_PTRDIFFT, TP_PTRDIFFT, T_SSIZET, T_SIZET, TP_SIZET,
        T_INTMAXT, T_UINTMAXT, TP_INTMAXT, TP_VOID, TP_CHAR, TP_SCHAR,
@@ -144,7 +144,7 @@
 static int     __sprint(FILE *, struct __suio *);
 #endif
 static int     __find_arguments(const CHAR_T *, va_list, union arg **);
-static int     __grow_type_table(int, enum typeid **, int *);
+static int     __grow_type_table(size_t, enum typeid **, size_t *);
 
 /*
  * Helper function for `fprintf to unbuffered unix file': creates a
@@ -1554,20 +1554,27 @@
 {
        CHAR_T *fmt;            /* format string */
        int ch;                 /* character from fmt */
-       int n, n2;              /* handy integer (short term usage) */
+       size_t n, n2;           /* handy index (short term usage) */
        CHAR_T *cp;             /* handy char pointer (short term usage) */
        int flags;              /* flags as above */
        enum typeid *typetable; /* table of types */
        enum typeid stattypetable [STATIC_ARG_TBL_SIZE];
-       int tablesize;          /* current size of type table */
-       int tablemax;           /* largest used index in table */
-       int nextarg;            /* 1-based argument index */
+       size_t tablesize;       /* current size of type table */
+       size_t tablemax;        /* largest used index in table */
+       size_t nextarg;         /* 1-based argument index */
+       size_t nitems;          /* number of items we picked from the stack */
 
        /*
         * Add an argument type to the table, expanding if necessary.
+        * Check for overflow.
         */
 #define ADDTYPE(type) \
        do { \
+               if (nextarg > SIZE_MAX / sizeof(**argtable)) { \
+                       if (typetable != stattypetable) \
+                               free(typetable); \
+                       return -1; \
+               } \
                if (nextarg >= tablesize) \
                        if (__grow_type_table(nextarg, &typetable, \
                            &tablesize) == -1) \
@@ -1575,6 +1582,7 @@
                if (nextarg > tablemax) \
                        tablemax = nextarg; \
                typetable[nextarg++] = type; \
+               nitems++; \
        } while (/*CONSTCOND*/0)
 
 #define        ADDSARG() \
@@ -1619,7 +1627,7 @@
                cp++; \
        } \
        if (*cp == '$') { \
-               int hold = nextarg; \
+               size_t hold = nextarg; \
                nextarg = n2; \
                ADDTYPE(T_INT); \
                nextarg = hold; \
@@ -1628,12 +1636,12 @@
                ADDTYPE(T_INT); \
        }
        fmt = (CHAR_T *)__UNCONST(fmt0);
+       memset(stattypetable, 0, sizeof(stattypetable));
        typetable = stattypetable;
        tablesize = STATIC_ARG_TBL_SIZE;
        tablemax = 0; 
        nextarg = 1;
-       for (n = 0; n < STATIC_ARG_TBL_SIZE; n++)
-               typetable[n] = T_UNUSED;
+       nitems = 1;
 
        /*
         * Scan the format for conversions (`%' character).
@@ -1795,13 +1803,30 @@
        }
 done:
        /*
+        * nitems contains the number of arguments we picked from the stack.
+        * If tablemax is larger, this means that some positional argument,
+        * tried to pick an argument the number of arguments possibly supplied.
+        * Since positional arguments are typically used to swap the order of
+        * the printf arguments and not to pick random arguments from strange
+        * positions in the stack, we assume that if the positional argument
+        * is trying to pick beyond the end of arguments, then this is wrong.
+        * Alternatively we could find a way to figure out when va_arg() runs
+        * out, but how to do that?
+        */
+       if (nitems < tablemax) {
+               if (typetable != stattypetable)
+                       free(typetable);
+               return -1;
+       }
+       /*
         * Build the argument table.
         */
        if (tablemax >= STATIC_ARG_TBL_SIZE) {
-               *argtable = (union arg *)
-                   malloc (sizeof (union arg) * (tablemax + 1));
-               if (*argtable == NULL)
+               *argtable = malloc(sizeof(**argtable) * (tablemax + 1));
+               if (*argtable == NULL) {
+                       free(typetable);
                        return -1;
+               }
        }
 
        (*argtable) [0].intarg = 0;
@@ -1892,7 +1917,7 @@
                }
        }
 
-       if ((typetable != NULL) && (typetable != stattypetable))
+       if (typetable != stattypetable)
                free (typetable);
        return 0;
 }
@@ -1901,28 +1926,27 @@
  * Increase the size of the type table.
  */
 static int
-__grow_type_table (int nextarg, enum typeid **typetable, int *tablesize)
+__grow_type_table (size_t nextarg, enum typeid **typetable, size_t *tablesize)
 {
        enum typeid *const oldtable = *typetable;
        const int oldsize = *tablesize;
        enum typeid *newtable;
-       int n, newsize = oldsize * 2;
+       size_t n, newsize = oldsize * 2;
 
        if (newsize < nextarg + 1)
                newsize = nextarg + 1;
        if (oldsize == STATIC_ARG_TBL_SIZE) {
-               if ((newtable = malloc(newsize * sizeof(enum typeid))) == NULL)
+               if ((newtable = malloc(newsize * sizeof(*newtable))) == NULL)
                        return -1;
-               memcpy(newtable, oldtable, oldsize * sizeof(enum typeid));
+               memcpy(newtable, oldtable, oldsize * sizeof(*newtable));
        } else {
-               newtable = realloc(oldtable, newsize * sizeof(enum typeid));
+               newtable = realloc(oldtable, newsize * sizeof(*newtable));
                if (newtable == NULL) {
                        free(oldtable);
                        return -1;
                }
        }
-       for (n = oldsize; n < newsize; n++)
-               newtable[n] = T_UNUSED;
+       memset(&newtable[oldsize], 0, (newsize - oldsize) * sizeof(*newtable));
 
        *typetable = newtable;
        *tablesize = newsize;