From mboxrd@z Thu Jan  1 00:00:00 1970
From: Alexander Viro <viro@math.psu.edu>
To: 9fans@cse.psu.edu
Subject: Re: [9fans] Plan 9 (in)security
In-Reply-To: <200105261954.f4QJsmj04287@ducky.net>
Message-ID: <Pine.GSO.4.21.0105261759130.1754-100000@weyl.math.psu.edu>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Date: Sat, 26 May 2001 18:47:46 -0400
Topicbox-Message-UUID: a8f5155c-eac9-11e9-9e20-41e7f4b1d025



On Sat, 26 May 2001, Mike Haertel wrote:

> Well, it turns out that convM2S() doesn't even enforce this.  It
> uses memmove() to move exactly NAMELEN bytes from the input buffer
> to the Fcall structure--whether or they contain a nul-terminated
> string or anything else.  It won't overrun its destination Fcall
> structure, but a clever attacker could probably arrange for data
> to be written into the Fcall structure that might later cause a
> file server using a strcpy() family function to go out of bounds
> and do who knows what.  A similar caveat applies to convM2D().
> For example, you might be able to crash or corrupt a file server
> by sending a wstat request in which the name, uid, gid, and perhaps
> following fields all contained no zero bytes.

Ehh... How about
                        ni = BGLONG(a+5);
                        font->fchar = malloc(ni*sizeof(FChar));
                        if(font->fchar == 0)
                                error("no memory for font");
                        memset(font->fchar, 0, ni*sizeof(FChar));
                        font->nfchar = ni;
                        font->ascent = a[9];

in devdraw.c? ni is received from user, sizeof(FChar) > 1, no sanity checks
on ni. Anyone who wouldn't see why that code is b0rken? And yes, it's
panicable - several lines below we have
                        ci = BGSHORT(a+9);
                        if(ci >= font->nfchar)
                                error(Eindex);
                        drawrectangle(&r, a+11);
                        drawpoint(&p, a+27);
                        memdraw(font->image, r, src, p, memopaque, p);
                        fc = &font->fchar[ci];
                        fc->minx = r.min.x;
IOW, have ni = ~0U / sizeof(FChar) + 42,  then ci = 69 and enjoy memory
corruption.

OK, it's not a security hole - you'd need to have access to /dev/draw on
CPU server to screw somebody else. However, having write(2) panicing the
box is a Bad Thing(tm).

Furrfu... It's not like that was something new. malloc(foo*sizeof(bar))
can return non-NULL and allocate less than array of foo elements - just
have the product slightly above the UINT_MAX.