Export to GitHub

memcached - issue #192

Crash when sending specially crafted packet


Posted on Mar 15, 2011 by Grumpy Camel

What steps will reproduce the problem? 1. Start memcached in TCP mode. For example:

$ ./memcached -v -p 11211 -U 0

  1. Send the specially crafted packet to it:

$ echo -en '\x80\x12\x00\x01\x08\x00\x00\x00\xff\xff\xff\xe8\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xff\xff\xff\xff\x01\x00\x00\x00\x00\x00\x00\x00\x00\x000\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00' | nc localhost 11211

What is the expected output? What do you see instead?

The expected output is an error message (since the packet is not valid). Instead, memcached segfaults. The backtrace in gdb is:

0 0x00007ffff76a93e2 in _wordcopy_bwd_dest_aligned (dstp=140737352380384, srcp=6497184, len=2305843009213693940) at wordcopy.c:392

1 0x00007ffff76a75e9 in memmove (dest=0x7ffff7e53053, src=<value optimized out>, len=18446744073709551581) at memmove.c:99

2 0x000000000040a165 in drive_machine (fd=<value optimized out>, which=<value optimized out>, arg=0x632200) at /usr/include/bits/string3.h:59

3 event_handler (fd=<value optimized out>, which=<value optimized out>, arg=0x632200) at memcached.c:3732

4 0x00007ffff7bc8194 in event_base_loop () from /usr/lib/libevent-1.4.so.2

5 0x000000000040db84 in worker_libevent (arg=0x61c0a0) at thread.c:245

6 0x00007ffff79ac971 in start_thread (arg=<value optimized out>) at pthread_create.c:304

7 0x00007ffff770892d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112

8 0x0000000000000000 in ?? ()

What version of the product are you using? On what operating system?

Memcached 1.4.5 / Ubuntu 10.10 x64

Please provide any additional information below.

The packet was generated automatically by a research testing tool under development at the Dependable Systems Laboratory, at the Swiss Federal Institute of Technology (EPFL), Switzerland (http://dslab.epfl.ch/).

Comment #1

Posted on Mar 15, 2011 by Happy Rhino

it is easy to fix with small patch, but in memcached internals there are so many functions without correct arguments cheking.

Attachments

Comment #2

Posted on Sep 14, 2011 by Happy Bear

The patch in comment one seems to do the trick. I hope this is fixed in the next version.

Comment #3

Posted on Feb 21, 2012 by Happy Rhino

Still working on Memcache 1.4.13

wget http://memcached.googlecode.com/files/memcached-1.4.13.tar.gz

tar xzvf memcached-1.4.13.tar.gz

cd memcached-1.4.13

./configure

make

make install

which memcached

/usr/local/bin/memcached

memcached -d -u nobody

echo -e "stats\n" | nc localhost 11211 | awk '/pid/ {print $3}'

28039

echo -en '\x80\x12\x00\x01\x08\x00\x00\x00\xff\xff\xff\xe8\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xff\xff\xff\xff\x01\x00\x00\x00\x00\x00\x00\x00\x00\x000\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00' | nc localhost 11211

dmesg | tail -n 1

[173531.976403] memcached[28042]: segfault at 7f11697c0ff0 ip 00007f1168ea5f6b sp 00007f1167528cc8 error 6 in libc-2.13.so[7f1168d68000+195000]


OS: 3.0.0-15-generic #26-Ubuntu SMP Fri Jan 20 17:23:00 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux Memcache 1.4.13

Comment #4

Posted on Feb 22, 2012 by Swift Elephant

i fixed this bugļ¼Œ how can i commit the patch? thank you.

Comment #5

Posted on Feb 22, 2012 by Swift Elephant

this is my pathed for this bug , thanks

Attachments

Comment #6

Posted on Mar 21, 2012 by Happy Bird

Memcached still crashes in version 1.4.7 with exploit given above.

Comment #7

Posted on Apr 29, 2013 by Happy Giraffe

Hello

WHEN YOU WILL FIX THIS BUG?

TWO YEARS !!!!!!!

OMG

Comment #8

Posted on Apr 29, 2013 by Helpful Lion

How many clients are actually sending that packet? Could it be that it's not affecting anyone? This is an open source project so you're more than welcome to supply a patch ;)

Comment #9

Posted on Apr 29, 2013 by Grumpy Camel

Why are patches in #1 and #5 not good enough?

Comment #10

Posted on Apr 29, 2013 by Helpful Lion

I haven't looked in detail how the packet looks. From looking at the patches I'd say that the one in 1 should at least go away because you shouldn't get that deep into the code with an ivalid packet. It should all be handled in the protocol layer.

None of the patches adds a test case in the test suite

Comment #11

Posted on Apr 30, 2013 by Massive Panda

Comment deleted

Comment #12

Posted on Apr 30, 2013 by Massive Panda

Please use CVE-2011-4971 for this issue.

Comment #13

Posted on Apr 30, 2013 by Helpful Bird

for what it's worth, I was hoping to get this in on the "next" release, but I haven't had time to untangle what's sitting in HEAD and roll in other bugs.

This bug, like many of the other open issues, has some shitty patches with no tests, no explanation, and this codebase is complicated enough that there're often better ways to fix things. A bulk of the "Security patches" I've had for memcached do nothing but open a different hole later on. I really do not have time for this bullshit.

I've had security patches on here where applying it actually opens a worse hole later on in the code: so these things end up being a pretty big burden and can take hours to validate.

I was also hoping to sidestep the issue entirely by releasing 1.5, which was largely rewritten, but that never happened. Every productive and sane committer ends up hired into a company which "loosely competes" with memcached, and is hard to secure time or motivation myself.

Now you fucking dinks have made me write a comment defending a lack of security fixes to a popular OSS daemon. I hope you can rub out a good one on this.

Let me know when someone stands up to write a test case and fucking prove their patch doesn't make things worse later on in the code. Stop cargo-culting patterns for security holes and do work like a fucking functional adult should.

Comment #14

Posted on Apr 30, 2013 by Happy Elephant

And this pals is why some people shouldn't be allowed to create open source projects

Comment #15

Posted on Apr 30, 2013 by Helpful Bird

I didn't create it: I'm just stupid enough to have tried to support it.

Comment #16

Posted on Apr 30, 2013 by Massive Panda

I am in firm defense of dormando on this one. If people would actually contribute a good patch with tests, this likely could have been closed long ago.

The reality is, this is only an issue for people stupid enough to have your memcached listening on a public network. Otherwise, how is someone malicious going to send that packet to my memcached? Secondly, it segfaults. That can cause a DOS. If your infrastructure has memcached on a public port AND also relies on it so much that your arch falls because memcached is restarted, you have bigger issues in your arch and need to rethink a lot of things.

And yes, he didn't create memcached. He has however been the only person doing any real work on a daemon that powers thousands of web applications world wide for several years now. The person that made it has moved on to Google and the other people that were putting a lot of work into it are doing their own noSQL memcached in the cloud kind of database thing.

So, kudos to Dormando for giving this bug all the attention it deserves.

Comment #17

Posted on May 1, 2013 by Massive Panda

Yes. Not the worst case scenario with this security vulnerability, but it can still affect availability in some environments. Please fix it "as soon as possible" when you have time. Thank you for maintaining this software. It's not always so easy :)

Comment #18

Posted on May 1, 2013 by Grumpy Camel

@dormando: I agree with your points and kudos for maintaining this software. I agree bad patches can hurt the codebase a lot, but I guess people had good intentions and the lack of clear patch submission guidelines makes this process a bit ambiguous.

@brian: (Disclaimer: I ain't a security expert, so take this with a grain of salt) I believe this vulnerability isn't useful by itself, but it can become a link in a chain of attacks, where the attacker first gains access to a memcached client machine (using another vulnerability), and then uses this bug to hijack the memcached process. Beside getting access to the host machine, the memcached server may hold potentially sensitive and private information, which could be leaked to the attacker.

PS: FYI, this bug was discovered using the Cloud9 automated testing engine, now available at http://cloud9.epfl.ch/ The tool is knowledgeable enough to generate test cases to expose the bug through a crash, but the real risk is that the bug can be exploited in a more elaborate way, e.g., inject and execute arbitrary code.

Comment #19

Posted on Jun 18, 2013 by Happy Bear

Attaching a fix (based on the previously posted patch on this bug) with a test. The patch is very simple, just checks if c->rlbytes is negative, which it should not be, so i am pretty show this is not going to break anything else anywhere.

Attachments

Comment #20

Posted on Jun 18, 2013 by Happy Bear

Maybe:

  • fprintf(stderr, "Invalid rlbytes to read: len %u\n", c->rlbytes);
  • fprintf(stderr, "Invalid rlbytes to read: len %d\n", c->rlbytes);

Comment #21

Posted on Jul 20, 2013 by Swift Kangaroo

Ok, so there is a patch. I found this thread because I am worried that memcached on my server may be getting attacked in some way. How can I apply this patch--I am server admin, but need a detailed explanation please.

Also, how soon will this patch make its way into cpanel's automatic updates? I am currently running memcached-1.4.15, and am not sure if it has been applied already, or if I should do it myself.

Comment #22

Posted on Nov 26, 2013 by Quick Hippo

The attached patches to this issue hide the problem but they do not solve it.

This is an unsigned to signed integer conversion problem. Inside the following functions: process_bin_sasl_auth process_bin_complete_sasl_auth process_bin_update process_bin_append_prepend

there is the following or a similar statement: int vlen = c->binary_header.request.bodylen - nkey;

The c->binary_header.request.bodylen is an unsigned int which if it is bigger than the INT_MAX and converted to a signed int will result to a negative number causing segfaults to memcached. The c->binary_header.request.bodylen is the request body length defined by the client request. Random bytes sent to the memcached may interpeted as a normal request with huge body data.

There are 2-3 different ways to solve this problem. This patch just add a check and reject requests which report huge body data.

Regards, Christos

Attachments

Comment #23

Posted on Nov 26, 2013 by Swift Kangaroo

I appreciate the patch, but do I need to install it if I am running memcached-1.4.15? How do I install this patch if I do need it?

Comment #24

Posted on Dec 9, 2013 by Helpful Bird

Adams: If you are in any sort of position where random people can connect to your memcached instance, many bad things can happen. Please do not do this.

Comment #25

Posted on Dec 9, 2013 by Helpful Bird

chtsanti: Could you please add tests for the specific issues your patch addresses, and resubmit under a new bug report (or just a pull request on github)?

I've merged sidhpurwala.huzaifa's change after some formatting fixes since it came with a test and wasn't as nuts as some of the original patches. I'm closing out this ticket, but I acknowledge that there are many things to fix in the codebase.

I'll repeat again: Almost all of the security patches I've seen submitted have been awful. They either change a harmless crash into an actual segmentation fault, are pointless cargo-culted changes, or simply lack tests.

It's really not that hard to do correctly. Channel some of that rage into writing a quick test and understanding the code a bit better, please.

Comment #26

Posted on Dec 10, 2013 by Quick Hippo

@dorma.....

The patch I attached is for this bug, not for a new or on other bug. I just believe it is better solution. I have a small description, of what this patch does, and why it is the correct solution to the problem in bug preamble and in my comment.

I do not understand why you are saying that there is not test case. The test case reported at the top of this page, when this bug reported: echo -en '\x80\x12\x00\x01\x08\x00\x00\x00\xff\xff\xff\xe8\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xff\xff\xff\xff\x01\x00\x00\x00\x00\x00\x00\x00\x00\x000\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00' | nc localhost 11211

Also the perl script in sidhpurwala.huzaifa's memcached-Fix-crash.txt file is a test case.

I'll repeat again: Almost all of the security patches I've seen submitted have been awful. They either change a harmless crash into an actual segmentation fault, are pointless cargo-culted changes, or simply lack tests.

It is project members responsibility to check and accept or reject patches.

I was just posted a patch here because I asked by a customer for this. He was worrying because in many cases, crackers do not need direct access to a server to crack it.

I still believe that my patch is a correct solution. The tests are not always enough, project developers should always examine the code. You can easily examine the code I am changing and decide if what I am claiming is correct or not.

In any case, accept or reject a patch is project developers responsibility.

Regards, chtsanti

Comment #27

Posted on Dec 10, 2013 by Grumpy Camel

Thank you Chtsanti, maybe they'll read this in 3 more months...I've been trying to obtain basic instructions for how to apply your patch. I am a mediocre server admin, and would need some detail. Any help would be appreciated.

Also, if they accept your patch do they pipe it into the new updates that would be done on my server automatically via CPanel?

Comment #28

Posted on Dec 10, 2013 by Helpful Bird

chtsanti:

I am talking about automated regression tests in the test suite. I don't know why this is so hard to understand. I rejected several patches in here because they lack test cases for the test suite, then I accepted one which included a test. I said explicitly why I was accepting it. Can you please write tests into the test suite with your patch? Then we have regression testing for future changes and it's much much easier to validate on multiple platforms than me doing that by hand.

Comment #29

Posted on Dec 10, 2013 by Helpful Bird

Scott; The bug is fixed in 1.4.16.

Status: Fixed

Labels:
Type-Defect Priority-Medium