remote stack overflow in memcached-1.1.11

[Brad Fitzpatrick]

Keys are limited to 250 bytes (251 with null) in other places already.

- Brad


On Thu, 22 Jul 2004, Larry Leszczynski wrote:

> Hi Brad -
>
> Do really long input keys get truncated to 250 characters before they hit
> this sprintf?  Even if the buffer is now big enough to fit a message with
> a 250 character key, it seems like it would be even safer still to change
> the sprintf to a snprintf, to make sure nothing ever gets written past the
> end of the temp buffer.
>
>
> Larry
>
>
> On Thu, 22 Jul 2004, Brad Fitzpatrick wrote:
>
> > Fix committed.  Thanks!
> >
> > To calm any fears for others:  I don't really consider this a security
> > problem, considering memcached makes no attempt to authorize clients.  If
> > you have a public-facing memcached server, you have bigger problems.
> >
> > More than anything, I consider this a robustness problem.  It's quite
> > legit (although perhaps odd) to have 250-length keys, and all the
> > memcached functionality should work (including the stats commands) when
> > using such length keys.
> >
> > If other want the fix, I just did this:
> >
> > $ cvs diff -r1.21 -r1.22 items.c
> > Index: items.c
> > ===================================================================
> > RCS file: /home/cvspub/wcmtools/memcached/items.c,v
> > retrieving revision 1.21
> > retrieving revision 1.22
> > diff -u -r1.21 -r1.22
> > --- items.c     31 Mar 2004 05:01:51 -0000      1.21
> > +++ items.c     22 Jul 2004 19:49:51 -0000      1.22
> > @@ -1,5 +1,5 @@
> >  /* -*- Mode: C; tab-width: 4; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> > -/* $Id: items.c,v 1.21 2004/03/31 05:01:51 avva Exp $ */
> > +/* $Id: items.c,v 1.22 2004/07/22 19:49:51 bradfitz Exp $ */
> >
> >  #include <sys/types.h>
> >  #include <sys/stat.h>
> > @@ -214,7 +214,7 @@
> >      item *it;
> >      int len;
> >      int shown = 0;
> > -    char temp[256];
> > +    char temp[512];
> >
> >      if (slabs_clsid > LARGEST_ID) return 0;
> >      it = heads[slabs_clsid];
> >
> >
> > - Brad
> >
> >
> >
> > On Thu, 22 Jul 2004, Andrei Nigmatulin wrote:
> >
> > > Hello,
> > >
> > > Just found generic stack overflow in memcached, it's easy reproducable:
> > >
> > > > telnet localhost 11211
> > > Trying 127.0.0.1...
> > > Connected to localhost.
> > > Escape character is '^]'.
> > > add
> > > qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq 0 0 0
> > > STORED
> > > stats cachedump 9 100000
> > > Connection closed by foreign host.
> > >
> > > memcached daemon dies with segmentation fault message.
> > >
> > > The bug lays here:
> > >
> > > char *item_cachedump(unsigned int slabs_clsid, unsigned int limit,
> > > unsigned int *bytes) {
> > > ...
> > >     char temp[256];
> > > ...
> > >         sprintf(temp, "ITEM %s [%u b; %lu s]\r\n", ITEM_key(it),
> > > it->nbytes - 2, it->time);
> > >
> > > While maximum key length is 250 bytes it is possible to overflow stack
> > > variable temp and may be even execute arbitrary code (not checked at
> > > this moment)
> > >
> > > --
> > > Andrei Nigmatulin
> > > GPG PUB KEY 6449830D
> > >
> > >
> >
>
>