PIping null to the server will crash it

[GoogleCodeExporter]

What steps will reproduce the problem?
1. do "cat /dev/zero | nc -q1 127.0.0.1 11211"
2. Wait a short while
3. Watch the server crash

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

The server should probably disconnect the client on such bad input

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

memcached 1.2.8

ubuntu jaunty

Please provide any additional information below.

Original issue reported on code.google.com by fallenpe...@gmail.com on 28 Oct 2009 at 1:10

[GoogleCodeExporter]

The problem here is that the client will try to spool the data it receives from 
the
client until it receives '\n' (indicating that it got the complete command). In 
this
case we never receive '\n' so we end up reallocating the input buffer until 
realloc
fails, causing the client connection to be shut down. My server didn't crash, 
but
that could be caused other mallocs succeeding (and failing in your case). There 
might
therefore be a path through the code that doesn't handle a malloc failure.

To protect us against someone eating up all of the memory in one buffer, I 
suggest
that we just shut down the connection if we receive more than KEY_MAX_LENGTH + 
let's
say 100 bytes (command, flags, length, cas etc) without seeing '\n'. 

Another part of the problem is try_read_network. in this function we try to 
read out
all of the data from the socket reallocating the input buffer to a bigger 
buffer if
there is more data available. In the test-case above we might never break out 
of this
loop, because one core can fill the buffer while the other core on the system 
is busy
reading and reallocating. As a workaround for this we should jump back out 
after a
low number of reallocs.

Proposed fix at: http://github.com/trondn/memcached/tree/issue_102

Original comment by trond.no...@gmail.com on 28 Oct 2009 at 10:55

• Changed state: Started

[GoogleCodeExporter]

This doesn't seem right.  Doesn't that make multiget not work, or am I not 
understanding?

I think the first, say, 16 bytes needs to look like a valid command at the very
least.  Your code makes sense for anything except get, so perhaps it'd be OK 
and get
can be special cased?

It'd also be good to do away with the "allow any arbitrary whitespace before
commands" stuff in the protocol.  That'd simplify it down to just checking the 
first
byte for most things.

Original comment by dsalli...@gmail.com on 28 Oct 2009 at 5:15

[GoogleCodeExporter]

Latest changes look good.  Thanks!  :)

Original comment by dsalli...@gmail.com on 29 Oct 2009 at 1:26

• Changed state: Fixed