mod_proxy_uwsgi: Stricter backend HTTP response parsing/validation

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1907980 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/changes-entries/proxy_uwsgi_response_validation.txt b/changes-entries/proxy_uwsgi_response_validation.txt
new file mode 100644 (file)
index 0000000..2cdb6c6
--- /dev/null
+++ b/changes-entries/proxy_uwsgi_response_validation.txt
@@ -0,0 +1,2 @@
+  *) mod_proxy_uwsgi: Stricter backend HTTP response parsing/validation.
+     [Yann Ylavic]

diff --git a/modules/proxy/mod_proxy_uwsgi.c b/modules/proxy/mod_proxy_uwsgi.c
index f9f3b9111b4939569bd3db799b281049df70958c..720dfa848715012cb7da59522d1fe4d2146b07dc 100644 (file)
--- a/modules/proxy/mod_proxy_uwsgi.c
+++ b/modules/proxy/mod_proxy_uwsgi.c
@@ -313,18 +313,16 @@ static int uwsgi_response(request_rec *r, proxy_conn_rec * backend,
     pass_bb = apr_brigade_create(r->pool, c->bucket_alloc);
 
     len = ap_getline(buffer, sizeof(buffer), rp, 1);
-
     if (len <= 0) {
-        /* oops */
+        /* invalid or empty */
         return HTTP_INTERNAL_SERVER_ERROR;
     }
-
     backend->worker->s->read += len;
-
-    if (len >= sizeof(buffer) - 1) {
-        /* oops */
+    if ((apr_size_t)len >= sizeof(buffer)) {
+        /* too long */
         return HTTP_INTERNAL_SERVER_ERROR;
     }
+
     /* Position of http status code */
     if (apr_date_checkmask(buffer, "HTTP/#.# ###*")) {
         status_start = 9;
@@ -333,8 +331,8 @@ static int uwsgi_response(request_rec *r, proxy_conn_rec * backend,
         status_start = 7;
     }
     else {
-        /* oops */
-        return HTTP_INTERNAL_SERVER_ERROR;
+        /* not HTTP */
+        return HTTP_BAD_GATEWAY;
     }
     status_end = status_start + 3;
 
@@ -354,21 +352,44 @@ static int uwsgi_response(request_rec *r, proxy_conn_rec * backend,
     }
     r->status_line = apr_pstrdup(r->pool, &buffer[status_start]);
 
-    /* start parsing headers */
+    /* parse headers */
     while ((len = ap_getline(buffer, sizeof(buffer), rp, 1)) > 0) {
+        if ((apr_size_t)len >= sizeof(buffer)) {
+            /* too long */
+            len = -1;
+            break;
+        }
         value = strchr(buffer, ':');
-        /* invalid header skip */
-        if (!value)
-            continue;
-        *value = '\0';
-        ++value;
+        if (!value) {
+            /* invalid header */
+            len = -1;
+            break;
+        }
+        *value++ = '\0';
+        if (*ap_scan_http_token(buffer)) {
+            /* invalid name */
+            len = -1;
+            break;
+        }
         while (apr_isspace(*value))
             ++value;
         for (end = &value[strlen(value) - 1];
              end > value && apr_isspace(*end); --end)
             *end = '\0';
+        if (*ap_scan_http_field_content(value)) {
+            /* invalid value */
+            len = -1;
+            break;
+        }
         apr_table_add(r->headers_out, buffer, value);
     }
+    if (len < 0) {
+        /* Reset headers, but not to NULL because things below the chain expect
+         * this to be non NULL e.g. the ap_content_length_filter.
+         */
+        r->headers_out = apr_table_make(r->pool, 1);
+        return HTTP_BAD_GATEWAY;
+    }
 
     if ((buf = apr_table_get(r->headers_out, "Content-Type"))) {
         ap_set_content_type(r, apr_pstrdup(r->pool, buf));