remote stack overflow in memcached-1.1.11

[Brad Fitzpatrick]

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