Skip to content

Commit 75cc836

Browse files
Trond Norbyedustin
Trond Norbye
authored andcommitted
Issue 102: Piping null to the server will crash it
1 parent 0731dc8 commit 75cc836

File tree

2 files changed

+46
-2
lines changed

2 files changed

+46
-2
lines changed

Diff for: memcached.c

+29-2
Original file line numberDiff line numberDiff line change
@@ -3127,9 +3127,27 @@ static int try_read_command(conn *c) {
31273127

31283128
if (c->rbytes == 0)
31293129
return 0;
3130+
31303131
el = memchr(c->rcurr, '\n', c->rbytes);
3131-
if (!el)
3132+
if (!el) {
3133+
if (c->rbytes > 1024) {
3134+
/*
3135+
* We didn't have a '\n' in the first k. This _has_ to be a
3136+
* large multiget, if not we should just nuke the connection.
3137+
*/
3138+
char *ptr = c->rcurr;
3139+
while (*ptr == ' ') { /* ignore leading whitespaces */
3140+
++ptr;
3141+
}
3142+
3143+
if (strcmp(ptr, "get ") && strcmp(ptr, "gets ")) {
3144+
conn_set_state(c, conn_closing);
3145+
return 1;
3146+
}
3147+
}
3148+
31323149
return 0;
3150+
}
31333151
cont = el + 1;
31343152
if ((el - c->rcurr) > 1 && *(el - 1) == '\r') {
31353153
el--;
@@ -3191,12 +3209,17 @@ static enum try_read_result try_read_udp(conn *c) {
31913209
* close.
31923210
* before reading, move the remaining incomplete fragment of a command
31933211
* (if any) to the beginning of the buffer.
3212+
*
3213+
* To protect us from someone flooding a connection with bogus data causing
3214+
* the connection to eat up all available memory, break out and start looking
3215+
* at the data I've got after a number of reallocs...
3216+
*
31943217
* @return enum try_read_result
31953218
*/
31963219
static enum try_read_result try_read_network(conn *c) {
31973220
enum try_read_result gotdata = READ_NO_DATA_RECEIVED;
31983221
int res;
3199-
3222+
int num_allocs = 0;
32003223
assert(c != NULL);
32013224

32023225
if (c->rcurr != c->rbuf) {
@@ -3207,6 +3230,10 @@ static enum try_read_result try_read_network(conn *c) {
32073230

32083231
while (1) {
32093232
if (c->rbytes >= c->rsize) {
3233+
if (num_allocs == 4) {
3234+
return gotdata;
3235+
}
3236+
++num_allocs;
32103237
char *new_rbuf = realloc(c->rbuf, c->rsize * 2);
32113238
if (!new_rbuf) {
32123239
if (settings.verbose > 0)

Diff for: testapp.c

+17
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,22 @@ static enum test_return test_issue_92(void) {
538538
return TEST_PASS;
539539
}
540540

541+
static enum test_return test_issue_102(void) {
542+
char buffer[4096];
543+
memset(buffer, ' ', sizeof(buffer));
544+
buffer[sizeof(buffer) - 1] = '\0';
545+
546+
close(sock);
547+
sock = connect_server("127.0.0.1", port, false);
548+
549+
send_ascii_command(buffer);
550+
/* verify that the server closed the connection */
551+
assert(read(sock, buffer, sizeof(buffer)) == 0);
552+
close(sock);
553+
sock = connect_server("127.0.0.1", port, false);
554+
return TEST_PASS;
555+
}
556+
541557
static enum test_return start_memcached_server(void) {
542558
server_pid = start_server(&port, false, 600);
543559
sock = connect_server("127.0.0.1", port, false);
@@ -1676,6 +1692,7 @@ struct testcase testcases[] = {
16761692
/* The following tests all run towards the same server */
16771693
{ "start_server", start_memcached_server },
16781694
{ "issue_92", test_issue_92 },
1695+
{ "issue_102", test_issue_102 },
16791696
{ "binary_noop", test_binary_noop },
16801697
{ "binary_quit", test_binary_quit },
16811698
{ "binary_quitq", test_binary_quitq },

0 commit comments

Comments
 (0)