CVE-2010-1152: Improper Input Validation (CWE-20)
value: memcached.c in memcached before 1.4.3 allows remote attackers to cause a denial of service (daemon hang or crash) via a long line that triggers excessive memory allocation.  NOTE: some of these details are obtained from third party information.
CWE description: The product receives input or data, but it does
        not validate or incorrectly validates that the input has the
        properties that are required to process the data safely and
        correctly.
Published: 2010-04-12T18:30Z
Repository: https://github.com/memcached/memcached
Language: C
Files changed: memcached.c, testapp.c
Commit date: 2009-10-28 18:09:00 -0700
Commit message: Issue 102: Piping null to the server will crash it

Diff: diff --git a/memcached.c b/memcached.c
index f6f0071..1789135 100644
--- a/memcached.c
+++ b/memcached.c
@@ -3125,13 +3125,31 @@ static int try_read_command(conn *c) {
     } else {
         char *el, *cont;
 
         if (c->rbytes == 0)
             return 0;
+
         el = memchr(c->rcurr, '\n', c->rbytes);
-        if (!el)
+        if (!el) {
+            if (c->rbytes > 1024) {
+                /*
+                 * We didn't have a '\n' in the first k. This _has_ to be a
+                 * large multiget, if not we should just nuke the connection.
+                 */
+                char *ptr = c->rcurr;
+                while (*ptr == ' ') { /* ignore leading whitespaces */
+                    ++ptr;
+                }
+
+                if (strcmp(ptr, "get ") && strcmp(ptr, "gets ")) {
+                    conn_set_state(c, conn_closing);
+                    return 1;
+                }
+            }
+
             return 0;
+        }
         cont = el + 1;
         if ((el - c->rcurr) > 1 && *(el - 1) == '\r') {
             el--;
         }
         *el = '\0';
@@ -3189,26 +3207,35 @@ static enum try_read_result try_read_udp(conn *c) {
 /*
  * read from network as much as we can, handle buffer overflow and connection
  * close.
  * before reading, move the remaining incomplete fragment of a command
  * (if any) to the beginning of the buffer.
+ *
+ * To protect us from someone flooding a connection with bogus data causing
+ * the connection to eat up all available memory, break out and start looking
+ * at the data I've got after a number of reallocs...
+ *
  * @return enum try_read_result
  */
 static enum try_read_result try_read_network(conn *c) {
     enum try_read_result gotdata = READ_NO_DATA_RECEIVED;
     int res;
-
+    int num_allocs = 0;
     assert(c != NULL);
 
     if (c->rcurr != c->rbuf) {
         if (c->rbytes != 0) /* otherwise there's nothing to copy */
             memmove(c->rbuf, c->rcurr, c->rbytes);
         c->rcurr = c->rbuf;
     }
 
     while (1) {
         if (c->rbytes >= c->rsize) {
+            if (num_allocs == 4) {
+                return gotdata;
+            }
+            ++num_allocs;
             char *new_rbuf = realloc(c->rbuf, c->rsize * 2);
             if (!new_rbuf) {
                 if (settings.verbose > 0)
                     fprintf(stderr, "Couldn't realloc input buffer\n");
                 c->rbytes = 0; /* ignore what we read */
diff --git a/testapp.c b/testapp.c
index b0491c3..5ff2a1f 100644
--- a/testapp.c
+++ b/testapp.c
@@ -536,10 +536,26 @@ static enum test_return test_issue_92(void) {
     close(sock);
     sock = connect_server("127.0.0.1", port, false);
     return TEST_PASS;
 }
 
+static enum test_return test_issue_102(void) {
+    char buffer[4096];
+    memset(buffer, ' ', sizeof(buffer));
+    buffer[sizeof(buffer) - 1] = '\0';
+
+    close(sock);
+    sock = connect_server("127.0.0.1", port, false);
+
+    send_ascii_command(buffer);
+    /* verify that the server closed the connection */
+    assert(read(sock, buffer, sizeof(buffer)) == 0);
+    close(sock);
+    sock = connect_server("127.0.0.1", port, false);
+    return TEST_PASS;
+}
+
 static enum test_return start_memcached_server(void) {
     server_pid = start_server(&port, false, 600);
     sock = connect_server("127.0.0.1", port, false);
     return TEST_PASS;
 }
@@ -1674,10 +1690,11 @@ struct testcase testcases[] = {
     { "issue_44", test_issue_44 },
     { "vperror", test_vperror },
     /* The following tests all run towards the same server */
     { "start_server", start_memcached_server },
     { "issue_92", test_issue_92 },
+    { "issue_102", test_issue_102 },
     { "binary_noop", test_binary_noop },
     { "binary_quit", test_binary_quit },
     { "binary_quitq", test_binary_quitq },
     { "binary_set", test_binary_set },
     { "binary_setq", test_binary_setq },

Vulnerable code:         if (!el)

Fixed code: 
        if (!el) {
            if (c->rbytes > 1024) {
                /*
                 * We didn't have a '\n' in the first k. This _has_ to be a
                 * large multiget, if not we should just nuke the connection.
                 */
                char *ptr = c->rcurr;
                while (*ptr == ' ') { /* ignore leading whitespaces */
                    ++ptr;
                }

                if (strcmp(ptr, "get ") && strcmp(ptr, "gets ")) {
                    conn_set_state(c, conn_closing);
                    return 1;
                }
            }

        }
 *
 * To protect us from someone flooding a connection with bogus data causing
 * the connection to eat up all available memory, break out and start looking
 * at the data I've got after a number of reallocs...
 *
    int num_allocs = 0;
            if (num_allocs == 4) {
                return gotdata;
            }
            ++num_allocs;
static enum test_return test_issue_102(void) {
    char buffer[4096];
    memset(buffer, ' ', sizeof(buffer));
    buffer[sizeof(buffer) - 1] = '\0';

    close(sock);
    sock = connect_server("127.0.0.1", port, false);

    send_ascii_command(buffer);
    /* verify that the server closed the connection */
    assert(read(sock, buffer, sizeof(buffer)) == 0);
    close(sock);
    sock = connect_server("127.0.0.1", port, false);
    return TEST_PASS;
}

    { "issue_102", test_issue_102 },