You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by yl...@apache.org on 2015/10/31 18:52:28 UTC
svn commit: r1711657 - /apr/apr/trunk/memcache/apr_memcache.c
Author: ylavic
Date: Sat Oct 31 17:52:28 2015
New Revision: 1711657
URL: http://svn.apache.org/viewvc?rev=1711657&view=rev
Log:
apr_memcache: Better validate the values' length provided by the memcached to
terminate the connection appropriately in getp() and avoid an infinite loop in
multigetp().
Reported/Proposed by: Jeffrey Crowell <jcrowell google.com>
Adapted/Committed by: ylavic
Modified:
apr/apr/trunk/memcache/apr_memcache.c
Modified: apr/apr/trunk/memcache/apr_memcache.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/memcache/apr_memcache.c?rev=1711657&r1=1711656&r2=1711657&view=diff
==============================================================================
--- apr/apr/trunk/memcache/apr_memcache.c (original)
+++ apr/apr/trunk/memcache/apr_memcache.c Sat Oct 31 17:52:28 2015
@@ -731,6 +731,26 @@ apr_memcache_replace(apr_memcache_t *mc,
}
+/*
+ * Parses a decimal size from size_str, returning the value in *size.
+ * Returns 1 if parsing was successful, 0 if parsing failed.
+ */
+static int parse_size(const char *size_str, apr_size_t *size)
+{
+ char *endptr;
+ long size_as_long;
+
+ errno = 0;
+ size_as_long = strtol(size_str, &endptr, 10);
+ if ((size_as_long < 0) || (errno != 0) || (endptr == size_str) ||
+ (endptr[0] != ' ' && (endptr[0] != '\r' || endptr[1] != '\n'))) {
+ return 0;
+ }
+
+ *size = (unsigned long)size_as_long;
+ return 1;
+}
+
APR_DECLARE(apr_status_t)
apr_memcache_getp(apr_memcache_t *mc,
apr_pool_t *p,
@@ -799,13 +819,10 @@ apr_memcache_getp(apr_memcache_t *mc,
}
length = apr_strtok(NULL, " ", &last);
- if (length) {
- len = strtol(length, (char **)NULL, 10);
- }
-
- if (len == 0 ) {
- *new_length = 0;
- *baton = NULL;
+ if (!length || !parse_size(length, &len)) {
+ ms_bad_conn(ms, conn);
+ apr_memcache_disable_server(mc, ms);
+ return APR_EGENERAL;
}
else {
apr_bucket_brigade *bbb;
@@ -813,7 +830,6 @@ apr_memcache_getp(apr_memcache_t *mc,
/* eat the trailing \r\n */
rv = apr_brigade_partition(conn->bb, len+2, &e);
-
if (rv != APR_SUCCESS) {
ms_bad_conn(ms, conn);
apr_memcache_disable_server(mc, ms);
@@ -823,17 +839,14 @@ apr_memcache_getp(apr_memcache_t *mc,
bbb = apr_brigade_split(conn->bb, e);
rv = apr_brigade_pflatten(conn->bb, baton, &len, p);
-
if (rv != APR_SUCCESS) {
ms_bad_conn(ms, conn);
- apr_memcache_disable_server(mc, ms);
return rv;
}
rv = apr_brigade_destroy(conn->bb);
if (rv != APR_SUCCESS) {
ms_bad_conn(ms, conn);
- apr_memcache_disable_server(mc, ms);
return rv;
}
@@ -851,14 +864,18 @@ apr_memcache_getp(apr_memcache_t *mc,
}
if (strncmp(MS_END, conn->buffer, MS_END_LEN) != 0) {
- rv = APR_EGENERAL;
+ ms_bad_conn(ms, conn);
+ apr_memcache_disable_server(mc, ms);
+ return APR_EGENERAL;
}
}
else if (strncmp(MS_END, conn->buffer, MS_END_LEN) == 0) {
rv = APR_NOTFOUND;
}
else {
- rv = APR_EGENERAL;
+ ms_bad_conn(ms, conn);
+ apr_memcache_disable_server(mc, ms);
+ return APR_EGENERAL;
}
ms_release_conn(ms, conn);
@@ -1361,74 +1378,68 @@ apr_memcache_multgetp(apr_memcache_t *mc
char *last;
char *data;
apr_size_t len = 0;
+ apr_bucket *e = NULL;
apr_strtok(conn->buffer, " ", &last); /* just the VALUE, ignore */
key = apr_strtok(NULL, " ", &last);
flags = apr_strtok(NULL, " ", &last);
-
-
length = apr_strtok(NULL, " ", &last);
- if (length) {
- len = strtol(length, (char **) NULL, 10);
+
+ if (!length || !parse_size(length, &len)) {
+ rv = APR_EGENERAL;
+ }
+ else {
+ /* eat the trailing \r\n */
+ rv = apr_brigade_partition(conn->bb, len+2, &e);
+ }
+ if (rv != APR_SUCCESS) {
+ apr_pollset_remove (pollset, &activefds[i]);
+ mget_conn_result(TRUE, FALSE, rv, mc, ms, conn,
+ server_query, values, server_queries);
+ queries_sent--;
+ continue;
}
value = apr_hash_get(values, key, strlen(key));
-
-
if (value) {
- if (len != 0) {
- apr_bucket_brigade *bbb;
- apr_bucket *e;
-
- /* eat the trailing \r\n */
- rv = apr_brigade_partition(conn->bb, len+2, &e);
-
- if (rv != APR_SUCCESS) {
- apr_pollset_remove (pollset, &activefds[i]);
- mget_conn_result(FALSE, FALSE, rv, mc, ms, conn,
- server_query, values, server_queries);
- queries_sent--;
- continue;
- }
-
- bbb = apr_brigade_split(conn->bb, e);
-
- rv = apr_brigade_pflatten(conn->bb, &data, &len, data_pool);
-
- if (rv != APR_SUCCESS) {
- apr_pollset_remove (pollset, &activefds[i]);
- mget_conn_result(FALSE, FALSE, rv, mc, ms, conn,
- server_query, values, server_queries);
- queries_sent--;
- continue;
- }
-
- rv = apr_brigade_destroy(conn->bb);
- if (rv != APR_SUCCESS) {
- apr_pollset_remove (pollset, &activefds[i]);
- mget_conn_result(FALSE, FALSE, rv, mc, ms, conn,
- server_query, values, server_queries);
- queries_sent--;
- continue;
- }
-
- conn->bb = bbb;
-
- value->len = len - 2;
- data[value->len] = '\0';
- value->data = data;
+ apr_bucket_brigade *bbb;
+
+ bbb = apr_brigade_split(conn->bb, e);
+
+ rv = apr_brigade_pflatten(conn->bb, &data, &len, data_pool);
+ if (rv != APR_SUCCESS) {
+ apr_pollset_remove (pollset, &activefds[i]);
+ mget_conn_result(TRUE, FALSE, rv, mc, ms, conn,
+ server_query, values, server_queries);
+ queries_sent--;
+ continue;
+ }
+
+ rv = apr_brigade_destroy(conn->bb);
+ if (rv != APR_SUCCESS) {
+ apr_pollset_remove (pollset, &activefds[i]);
+ mget_conn_result(TRUE, FALSE, rv, mc, ms, conn,
+ server_query, values, server_queries);
+ queries_sent--;
+ continue;
}
-
+
+ conn->bb = bbb;
+
+ value->len = len - 2;
+ data[value->len] = '\0';
+ value->data = data;
+
value->status = rv;
value->flags = atoi(flags);
-
+
/* stay on the server */
i--;
-
}
else {
- /* TODO: Server Sent back a key I didn't ask for or my
- * hash is corrupt */
+ /* Server Sent back a key I didn't ask for or my
+ * hash is corrupt */
+ rv = APR_EGENERAL;
}
}
else if (strncmp(MS_END, conn->buffer, MS_END_LEN) == 0) {
@@ -1436,14 +1447,18 @@ apr_memcache_multgetp(apr_memcache_t *mc
apr_pollset_remove (pollset, &activefds[i]);
ms_release_conn(ms, conn);
apr_hash_set(server_queries, &ms, sizeof(ms), NULL);
-
queries_sent--;
}
else {
/* unknown reply? */
rv = APR_EGENERAL;
}
-
+ if (rv != APR_SUCCESS) {
+ apr_pollset_remove (pollset, &activefds[i]);
+ mget_conn_result(TRUE, FALSE, rv, mc, ms, conn,
+ server_query, values, server_queries);
+ queries_sent--;
+ }
} /* /for */
} /* /while */