remote stack overflow in memcached-1.1.11

[Larry Leszczynski]

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
> >
> >
>