Attachment 22841 Details for Bug 59087

Attachment which was added to the mail

zlib.attach (text/plain), 6.34 KB, created by Sebastian Krahmer on 2004-08-23 17:30:14 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Sebastian Krahmer

Created: 2004-08-23 17:30:14 UTC

Size: 6.34 KB

patch

obsolete

>>From broonie@sirena.org.uk Mon Aug 23 07:28:28 2004
>Date: Fri, 11 Jun 2004 22:46:48 +0100
>From: Mark Brown <broonie@sirena.org.uk>
>To: zlib@gzip.org
>Subject: Crash in inflate()
>Message-ID: <20040611214648.GA18139@sirena.org.uk>
>Mime-Version: 1.0
>Content-Type: multipart/mixed; boundary="Dxnq1zWXvFF0Q93v"
>Content-Disposition: inline
>X-Cookie: To program is to be.
>User-Agent: Mutt/1.5.6+20040523i
>Status: RO
>
>
>--Dxnq1zWXvFF0Q93v
>Content-Type: text/plain; charset=us-ascii
>Content-Disposition: inline
>
>I previously mentioned a bug that had been reported against the Debian
>package of zlib 1.2.1.1 where the library crashes on some input.  The
>bug log is at http://bugs.debian.org/252253.  I've obtained one of the
>offending files and have hopefully analyzed the bug.  The patch below
>and attached fixes the problem for me.  
>
>The source of the problem appears to be that throughout the inflate()
>function the standard way to handle a detected error is:
>
>	strm->msg = (char *)"Error message";
>	strm->mode = BSD;
>	break;
>
>However, while processing the CODELENS state there are a couple of cases
>where an error can be detected inside a while loop so this idiom doesn't
>exit the main processing but instead only exits the while loop.  This
>causes the code to continue into inflate_trees() and potentially crash
>on uninitialised values in the lens array[1].  The fix below replaces
>the break statement with a goto statement that does the right thing.
>
>A similar problem appears to exist in infback.c so I've corrected that
>too but not tested.  Looking at the code the same issue will also exist
>in the released 1.2.1.
>
>I suspect this bug may have security implications given that external
>input can cause a crash.
>
>[1] The crash that was seen was when lens[sym] was a very high value in
>    line 108 of inftrees.c:
>
>	for (sym = 0; sym < codes; sym++)
>        	count[lens[sym]]++;
>
>    and can also be avoided by initialising lens to all zeros in
>    inflateInit() but that is something of a bodge.  Still, it might be
>    an idea to do this for defense in depth - I don't understand the
>    code well enough to say for sure but my instincts tell me it's a
>    good idea.
>
>diff -urN zlib-1.2.1.1.orig/infback.c zlib-inflate-bug/infback.c
>--- zlib-1.2.1.1.orig/infback.c	2003-08-12 00:48:06.000000000 +0100
>+++ zlib-inflate-bug/infback.c	2004-06-11 22:13:09.000000000 +0100
>@@ -404,7 +404,7 @@
>                         if (state->have == 0) {
>                             strm->msg = (char *)"invalid bit length repeat";
>                             state->mode = BAD;
>-                            break;
>+			    goto inf_leave;
>                         }
>                         len = (unsigned)(state->lens[state->have - 1]);
>                         copy = 3 + BITS(2);
>@@ -427,7 +427,7 @@
>                     if (state->have + copy > state->nlen + state->ndist) {
>                         strm->msg = (char *)"invalid bit length repeat";
>                         state->mode = BAD;
>-                        break;
>+                        goto inf_leave;
>                     }
>                     while (copy--)
>                         state->lens[state->have++] = (unsigned short)len;
>diff -urN zlib-1.2.1.1.orig/inflate.c zlib-inflate-bug/inflate.c
>--- zlib-1.2.1.1.orig/inflate.c	2003-10-26 06:15:36.000000000 +0000
>+++ zlib-inflate-bug/inflate.c	2004-06-11 22:12:29.000000000 +0100
>@@ -831,7 +831,7 @@
>                         if (state->have == 0) {
>                             strm->msg = (char *)"invalid bit length repeat";
>                             state->mode = BAD;
>-                            break;
>+                            goto inf_leave;
>                         }
>                         len = state->lens[state->have - 1];
>                         copy = 3 + BITS(2);
>@@ -854,7 +854,7 @@
>                     if (state->have + copy > state->nlen + state->ndist) {
>                         strm->msg = (char *)"invalid bit length repeat";
>                         state->mode = BAD;
>-                        break;
>+                        goto inf_leave;
>                     }
>                     while (copy--)
>                         state->lens[state->have++] = (unsigned short)len;
>
>-- 
>"You grabbed my hand and we fell into it, like a daydream - or a fever."
>
>--Dxnq1zWXvFF0Q93v
>Content-Type: text/plain; charset=us-ascii
>Content-Disposition: attachment; filename="inflate-bug.diff"
>
>diff -urN zlib-1.2.1.1.orig/infback.c zlib-inflate-bug/infback.c
>--- zlib-1.2.1.1.orig/infback.c	2003-08-12 00:48:06.000000000 +0100
>+++ zlib-inflate-bug/infback.c	2004-06-11 22:13:09.000000000 +0100
>@@ -404,7 +404,7 @@
>                         if (state->have == 0) {
>                             strm->msg = (char *)"invalid bit length repeat";
>                             state->mode = BAD;
>-                            break;
>+			    goto inf_leave;
>                         }
>                         len = (unsigned)(state->lens[state->have - 1]);
>                         copy = 3 + BITS(2);
>@@ -427,7 +427,7 @@
>                     if (state->have + copy > state->nlen + state->ndist) {
>                         strm->msg = (char *)"invalid bit length repeat";
>                         state->mode = BAD;
>-                        break;
>+                        goto inf_leave;
>                     }
>                     while (copy--)
>                         state->lens[state->have++] = (unsigned short)len;
>diff -urN zlib-1.2.1.1.orig/inflate.c zlib-inflate-bug/inflate.c
>--- zlib-1.2.1.1.orig/inflate.c	2003-10-26 06:15:36.000000000 +0000
>+++ zlib-inflate-bug/inflate.c	2004-06-11 22:12:29.000000000 +0100
>@@ -831,7 +831,7 @@
>                         if (state->have == 0) {
>                             strm->msg = (char *)"invalid bit length repeat";
>                             state->mode = BAD;
>-                            break;
>+                            goto inf_leave;
>                         }
>                         len = state->lens[state->have - 1];
>                         copy = 3 + BITS(2);
>@@ -854,7 +854,7 @@
>                     if (state->have + copy > state->nlen + state->ndist) {
>                         strm->msg = (char *)"invalid bit length repeat";
>                         state->mode = BAD;
>-                        break;
>+                        goto inf_leave;
>                     }
>                     while (copy--)
>                         state->lens[state->have++] = (unsigned short)len;
>
>--Dxnq1zWXvFF0Q93v--
>

Actions: View

Attachments on bug 59087: 22841 | 22871 | 22872 | 22894