Buffer overflow in "!" handling?

Buffer overflow in "!" handling?

[DragonK]

Hello,

I've stumbled upon a buffer overflow in zsh 4.3.9 (and 4.3.6) related
to the handling of the "!" character in the command line (Linux).

It's triggerable by typing "!AAAAAAAAA...A" (lots of A's) at the zsh
prompt (works better if zsh is compiled with stack protection,
otherwise a lot of A's are needed :) ).

A quick look at the code indicates the problem to be in hist.c,
function histsubchar(), where buf[256] is getting overflowed (*ptr is
used to write to the buffer, but no check is made to see if ptr passed
the end of buf).  I might be wrong though, I only took a couple of
minutes to look at the code.

Re: Buffer overflow in "!" handling?

[Peter Stephenson]

On Wed, 25 Feb 2009 11:42:50 +0200
DragonK <dragonk@gmail.com> wrote:
> I've stumbled upon a buffer overflow in zsh 4.3.9 (and 4.3.6) related
> to the handling of the "!" character in the command line (Linux).
> 
> It's triggerable by typing "!AAAAAAAAA...A" (lots of A's) at the zsh
> prompt (works better if zsh is compiled with stack protection,
> otherwise a lot of A's are needed :) ).
> 
> A quick look at the code indicates the problem to be in hist.c,
> function histsubchar(), where buf[256] is getting overflowed (*ptr is
> used to write to the buffer, but no check is made to see if ptr passed
> the end of buf).  I might be wrong though, I only took a couple of
> minutes to look at the code.

You're right, that's nasty.  See if you can get it to happen with this...

Index: Src/hist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/hist.c,v
retrieving revision 1.86
diff -u -r1.86 hist.c
--- Src/hist.c	25 Nov 2008 18:39:04 -0000	1.86
+++ Src/hist.c	25 Feb 2009 10:24:08 -0000
@@ -394,9 +394,10 @@
     zlong ev;
     static int marg = -1;
     static zlong mev = -1;
-    char buf[256], *ptr;
+    char *buf, *ptr;
     char *sline;
     Histent ehist;
+    size_t buflen;
 
     /* look, no goto's */
     if (isfirstch && c == hatchar) {
@@ -445,7 +446,7 @@
 	    return bangchar;
 	}
 	cflag = 0;
-	ptr = buf;
+	ptr = buf = zhalloc(buflen = 265);
 
 	/* get event number */
 
@@ -455,8 +456,14 @@
 		c = ingetc();
 		if (c == '?' || c == '\n' || lexstop)
 		    break;
-		else
+		else {
 		    *ptr++ = c;
+		    if (ptr == buf + buflen) {
+			buf = hrealloc(buf, buflen, 2 * buflen);
+			ptr = buf + buflen;
+			buflen *= 2;
+		    }
+		}
 	    }
 	    if (c != '\n' && !lexstop)
 		c = ingetc();
@@ -484,6 +491,11 @@
 			break;
 		}
 		*ptr++ = c;
+		if (ptr == buf + buflen) {
+		    buf = hrealloc(buf, buflen, 2 * buflen);
+		    ptr = buf + buflen;
+		    buflen *= 2;
+		}
 		if (c == '#' || c == bangchar) {
 		    c = ingetc();
 		    break;


-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070

Re: Buffer overflow in "!" handling?

[DragonK]

>
> You're right, that's nasty.  See if you can get it to happen with this...
>

I've applied the patch and it seems to work now; as far as I
understand from the comments in mem.c, memory allocated with zhalloc()
doesn't need to be explicitly free()d, right?

Re: Buffer overflow in "!" handling?

[Peter Stephenson]

DragonK wrote:
> > You're right, that's nasty. See if you can get it to happen with this...
> 
> I've applied the patch and it seems to work now; as far as I
> understand from the comments in mem.c, memory allocated with zhalloc()
> doesn't need to be explicitly free()d, right?

Yes, that's correct; the heap of memory is popped in one go when we
return to the top level of processing.  The hrealloc() is a bit of a
hack... we're not really reallocating heap most of the time, we're just
allocating more somewhere else, but from the API point of view it's the
simplest thing to do in the rare cases where we really need more than
256 words.

Thanks for looking.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070