[src/trunk]: src/usr.bin Do proper input validation without penalizing perfor...

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

details:   https://anonhg.NetBSD.org/src/rev/886c12dee80a
branches:  trunk
changeset: 768426:886c12dee80a
user:      joerg <joerg%NetBSD.org@localhost>
date:      Tue Aug 16 13:55:01 2011 +0000

description:
Do proper input validation without penalizing performance.

diffstat:

 usr.bin/compress/zopen.c   |  40 ++++++++++++++++++++--------------------
 usr.bin/gzip/zuncompress.c |  37 ++++++++++++++++++++-----------------
 2 files changed, 40 insertions(+), 37 deletions(-)

diffs (175 lines):

diff -r ab278dd2277f -r 886c12dee80a usr.bin/compress/zopen.c
--- a/usr.bin/compress/zopen.c  Tue Aug 16 13:15:04 2011 +0000
+++ b/usr.bin/compress/zopen.c  Tue Aug 16 13:55:01 2011 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: zopen.c,v 1.14 2011/08/16 03:24:47 christos Exp $      */
+/*     $NetBSD: zopen.c,v 1.15 2011/08/16 13:55:01 joerg Exp $ */
 
 /*-
  * Copyright (c) 1985, 1986, 1992, 1993
@@ -37,7 +37,7 @@
 #if 0
 static char sccsid[] = "@(#)zopen.c    8.1 (Berkeley) 6/27/93";
 #else
-static char rcsid[] = "$NetBSD: zopen.c,v 1.14 2011/08/16 03:24:47 christos Exp $";
+static char rcsid[] = "$NetBSD: zopen.c,v 1.15 2011/08/16 13:55:01 joerg Exp $";
 #endif
 #endif /* LIBC_SCCS and not lint */
 
@@ -486,7 +486,7 @@
        block_compress = maxbits & BLOCK_MASK;
        maxbits &= BIT_MASK;
        maxmaxcode = 1L << maxbits;
-       if (maxbits > BITS) {
+       if (maxbits > BITS || maxbits < 12) {
                errno = EFTYPE;
                return (-1);
        }
@@ -497,14 +497,7 @@
                tab_suffixof(code) = (char_type) code;
        }
        free_ent = block_compress ? FIRST : 256;
-
-       finchar = oldcode = getcode(zs);
-       if (oldcode == -1)      /* EOF already? */
-               return (0);     /* Get out of here */
-
-       /* First code must be 8 bits = char. */
-       *bp++ = (u_char)finchar;
-       count--;
+       oldcode = -1;
        stackp = de_stack;
 
        while ((code = getcode(zs)) > -1) {
@@ -513,24 +506,31 @@
                        for (code = 255; code >= 0; code--)
                                tab_prefixof(code) = 0;
                        clear_flg = 1;
-                       free_ent = FIRST - 1;
-                       if ((code = getcode(zs)) == -1) /* O, untimely death! */
-                               break;
+                       free_ent = FIRST;
+                       oldcode = -1;
+                       continue;
                }
                incode = code;
 
-               /* Special case FOR kWkWk string. */
+               /* Special case for kWkWk string. */
                if (code >= free_ent) {
+                       if (code > free_ent || oldcode == -1) {
+                               /* Bad stream. */
+                               errno = EINVAL;
+                               return (-1);
+                       }
                        *stackp++ = finchar;
                        code = oldcode;
                }
+               /*
+                * The above condition ensures that code < free_ent.
+                * The construction of tab_prefixof in turn guarantees that
+                * each iteration decreases code and therefore stack usage is
+                * bound by 1 << BITS - 256.
+                */
 
                /* Generate output characters in reverse order. */
                while (code >= 256) {
-                       if (stackp - de_stack >= HSIZE - 1) {
-                               errno = EOVERFLOW;
-                               return -1;
-                       }
                        *stackp++ = tab_suffixof(code);
                        code = tab_prefixof(code);
                }
@@ -544,7 +544,7 @@
                } while (stackp > de_stack);
 
                /* Generate the new entry. */
-               if ((code = free_ent) < maxmaxcode) {
+               if ((code = free_ent) < maxmaxcode && oldcode != -1) {
                        tab_prefixof(code) = (u_short) oldcode;
                        tab_suffixof(code) = finchar;
                        free_ent = code + 1;
diff -r ab278dd2277f -r 886c12dee80a usr.bin/gzip/zuncompress.c
--- a/usr.bin/gzip/zuncompress.c        Tue Aug 16 13:15:04 2011 +0000
+++ b/usr.bin/gzip/zuncompress.c        Tue Aug 16 13:55:01 2011 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: zuncompress.c,v 1.10 2011/08/16 03:25:34 christos Exp $ */
+/*     $NetBSD: zuncompress.c,v 1.11 2011/08/16 13:55:02 joerg Exp $ */
 
 /*-
  * Copyright (c) 1985, 1986, 1992, 1993
@@ -246,7 +246,7 @@
        zs->zs_block_compress = zs->zs_maxbits & BLOCK_MASK;
        zs->zs_maxbits &= BIT_MASK;
        zs->zs_maxmaxcode = 1L << zs->zs_maxbits;
-       if (zs->zs_maxbits > BITS) {
+       if (zs->zs_maxbits > BITS || zs->zs_maxbits < 12) {
                errno = EFTYPE;
                return (-1);
        }
@@ -258,13 +258,7 @@
        }
        zs->zs_free_ent = zs->zs_block_compress ? FIRST : 256;
 
-       zs->u.r.zs_finchar = zs->u.r.zs_oldcode = getcode(zs);
-       if (zs->u.r.zs_oldcode == -1)   /* EOF already? */
-               return (0);     /* Get out of here */
-
-       /* First code must be 8 bits = char. */
-       *bp++ = (u_char)zs->u.r.zs_finchar;
-       count--;
+       zs->u.r.zs_oldcode = -1;
        zs->u.r.zs_stackp = de_stack;
 
        while ((zs->u.r.zs_code = getcode(zs)) > -1) {
@@ -274,24 +268,32 @@
                            zs->u.r.zs_code--)
                                tab_prefixof(zs->u.r.zs_code) = 0;
                        zs->zs_clear_flg = 1;
-                       zs->zs_free_ent = FIRST - 1;
-                       if ((zs->u.r.zs_code = getcode(zs)) == -1)      /* O, untimely death! */
-                               break;
+                       zs->zs_free_ent = FIRST;
+                       zs->u.r.zs_oldcode = -1;
+                       continue;
                }
                zs->u.r.zs_incode = zs->u.r.zs_code;
 
                /* Special case for KwKwK string. */
                if (zs->u.r.zs_code >= zs->zs_free_ent) {
+                       if (zs->u.r.zs_code > zs->zs_free_ent ||
+                           zs->u.r.zs_oldcode == -1) {
+                               /* Bad stream. */
+                               errno = EINVAL;
+                               return (-1);
+                       }
                        *zs->u.r.zs_stackp++ = zs->u.r.zs_finchar;
                        zs->u.r.zs_code = zs->u.r.zs_oldcode;
                }
+               /*
+                * The above condition ensures that code < free_ent.
+                * The construction of tab_prefixof in turn guarantees that
+                * each iteration decreases code and therefore stack usage is
+                * bound by 1 << BITS - 256.
+                */
 
                /* Generate output characters in reverse order. */
                while (zs->u.r.zs_code >= 256) {
-                       if (zs->u.r.zs_stackp - de_stack >= HSIZE - 1) {
-                               errno = EOVERFLOW;
-                               return -1;
-                       }
                        *zs->u.r.zs_stackp++ = tab_suffixof(zs->u.r.zs_code);
                        zs->u.r.zs_code = tab_prefixof(zs->u.r.zs_code);
                }
@@ -305,7 +307,8 @@
                } while (zs->u.r.zs_stackp > de_stack);
 
                /* Generate the new entry. */
-               if ((zs->u.r.zs_code = zs->zs_free_ent) < zs->zs_maxmaxcode) {
+               if ((zs->u.r.zs_code = zs->zs_free_ent) < zs->zs_maxmaxcode &&
+                   zs->u.r.zs_oldcode != -1) {
                        tab_prefixof(zs->u.r.zs_code) = (u_short) zs->u.r.zs_oldcode;
                        tab_suffixof(zs->u.r.zs_code) = zs->u.r.zs_finchar;
                        zs->zs_free_ent = zs->u.r.zs_code + 1;