Module Name: src
Committed By: joerg
Date: Tue Aug 16 13:55:02 UTC 2011
Modified Files:
src/usr.bin/compress: zopen.c
src/usr.bin/gzip: zuncompress.c
Log Message:
Do proper input validation without penalizing performance.
To generate a diff of this commit:
cvs rdiff -u -r1.14 -r1.15 src/usr.bin/compress/zopen.c
cvs rdiff -u -r1.10 -r1.11 src/usr.bin/gzip/zuncompress.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/usr.bin/compress/zopen.c
diff -u src/usr.bin/compress/zopen.c:1.14 src/usr.bin/compress/zopen.c:1.15
--- src/usr.bin/compress/zopen.c:1.14 Tue Aug 16 03:24:47 2011
+++ src/usr.bin/compress/zopen.c Tue Aug 16 13:55:01 2011
@@ -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;
Index: src/usr.bin/gzip/zuncompress.c
diff -u src/usr.bin/gzip/zuncompress.c:1.10 src/usr.bin/gzip/zuncompress.c:1.11
--- src/usr.bin/gzip/zuncompress.c:1.10 Tue Aug 16 03:25:34 2011
+++ src/usr.bin/gzip/zuncompress.c Tue Aug 16 13:55:02 2011
@@ -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;
