Merge r1908827, r1908838 from trunk:

mod_proxy: Check for space/ctrls in nocanon path/urls before forwarding.



Follow up to r1908827: CHANGES entry.
Submitted by: ylavic
Reviewed by: ylavic, rpluem, covener


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1908865 13f79535-47bb-0310-9956-ffa450edef68

9 files changed, 104 insertions, 59 deletions

diff --git a/STATUS b/STATUS
index 13871e1a9c..e64813a234 100644
--- a/STATUS
+++ b/STATUS
@@ -151,17 +151,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  *) mod_proxy: Check before forwarding that a nocanon path has not been
-                rewritten with spaces during processing.
-     trunk patch: https://svn.apache.org/r1908827
-                  https://svn.apache.org/r1908838
-     2.4.x patch: svn merge -c 1908827,1908838 ^/httpd/httpd/trunk .
-                  (convenience gh diff/PR below, but missing CHANGES entry..)
-                  https://patch-diff.githubusercontent.com/raw/apache/httpd/pull/354.diff
-                  https://github.com/apache/httpd/pull/354
-     +1: ylavic, rpluem, covener
-     rpluem says: svn merge should work fine once the backport above this one
-     is in.
 
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
diff --git a/changes-entries/proxy_check_path.txt b/changes-entries/proxy_check_path.txt
new file mode 100644
index 0000000000..f753e3a6bd
--- /dev/null
+++ b/changes-entries/proxy_check_path.txt
@@ -0,0 +1,3 @@
+  *) mod_proxy: Check before forwarding that a nocanon path has not been
+     rewritten with spaces during processing.  [Yann Ylavic]
+
diff --git a/modules/http2/mod_proxy_http2.c b/modules/http2/mod_proxy_http2.c
index 8af0a34165..5abccab097 100644
--- a/modules/http2/mod_proxy_http2.c
+++ b/modules/http2/mod_proxy_http2.c
@@ -164,26 +164,31 @@ static int proxy_http2_canon(request_rec *r, char *url)
 
             path = ap_proxy_canonenc_ex(r->pool, url, (int)strlen(url),
                                         enc_path, flags, r->proxyreq);
+            if (!path) {
+                return HTTP_BAD_REQUEST;
+            }
             search = r->args;
         }
-        if (search && *ap_scan_vchar_obstext(search)) {
-            /*
-             * We have a raw control character or a ' ' in r->args.
-             * Correct encoding was missed.
-             */
-            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10412)
-                          "To be forwarded query string contains control "
-                          "characters or spaces");
-            return HTTP_FORBIDDEN;
-        }
         break;
     case PROXYREQ_PROXY:
         path = url;
         break;
     }
-
-    if (path == NULL) {
-        return HTTP_BAD_REQUEST;
+    /*
+     * If we have a raw control character or a ' ' in nocanon path or
+     * r->args, correct encoding was missed.
+     */
+    if (path == url && *ap_scan_vchar_obstext(path)) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10420)
+                      "To be forwarded path contains control "
+                      "characters or spaces");
+        return HTTP_FORBIDDEN;
+    }
+    if (search && *ap_scan_vchar_obstext(search)) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10412)
+                      "To be forwarded query string contains control "
+                      "characters or spaces");
+        return HTTP_FORBIDDEN;
     }
 
     if (port != def_port) {
diff --git a/modules/proxy/mod_proxy_ajp.c b/modules/proxy/mod_proxy_ajp.c
index f5b25662e1..65773ce768 100644
--- a/modules/proxy/mod_proxy_ajp.c
+++ b/modules/proxy/mod_proxy_ajp.c
@@ -75,20 +75,27 @@ static int proxy_ajp_canon(request_rec *r, char *url)
 
         path = ap_proxy_canonenc_ex(r->pool, url, strlen(url), enc_path, flags,
                                     r->proxyreq);
+        if (!path) {
+            return HTTP_BAD_REQUEST;
+        }
         search = r->args;
     }
+    /*
+     * If we have a raw control character or a ' ' in nocanon path or
+     * r->args, correct encoding was missed.
+     */
+    if (path == url && *ap_scan_vchar_obstext(path)) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10418)
+                      "To be forwarded path contains control "
+                      "characters or spaces");
+        return HTTP_FORBIDDEN;
+    }
     if (search && *ap_scan_vchar_obstext(search)) {
-        /*
-         * We have a raw control character or a ' ' in r->args.
-         * Correct encoding was missed.
-         */
          ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10406)
                        "To be forwarded query string contains control "
                        "characters or spaces");
          return HTTP_FORBIDDEN;
     }
-    if (path == NULL)
-        return HTTP_BAD_REQUEST;
 
     if (port != def_port)
          apr_snprintf(sport, sizeof(sport), ":%d", port);
diff --git a/modules/proxy/mod_proxy_balancer.c b/modules/proxy/mod_proxy_balancer.c
index a3db6dcd9d..b8b452d0bf 100644
--- a/modules/proxy/mod_proxy_balancer.c
+++ b/modules/proxy/mod_proxy_balancer.c
@@ -112,20 +112,27 @@ static int proxy_balancer_canon(request_rec *r, char *url)
 
         path = ap_proxy_canonenc_ex(r->pool, url, strlen(url), enc_path, flags,
                                     r->proxyreq);
+        if (!path) {
+            return HTTP_BAD_REQUEST;
+        }
         search = r->args;
     }
+    /*
+     * If we have a raw control character or a ' ' in nocanon path or
+     * r->args, correct encoding was missed.
+     */
+    if (path == url && *ap_scan_vchar_obstext(path)) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10416)
+                      "To be forwarded path contains control "
+                      "characters or spaces");
+        return HTTP_FORBIDDEN;
+    }
     if (search && *ap_scan_vchar_obstext(search)) {
-        /*
-         * We have a raw control character or a ' ' in r->args.
-         * Correct encoding was missed.
-         */
          ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10407)
                        "To be forwarded query string contains control "
                        "characters or spaces");
          return HTTP_FORBIDDEN;
     }
-    if (path == NULL)
-        return HTTP_BAD_REQUEST;
 
     r->filename = apr_pstrcat(r->pool, "proxy:" BALANCER_PREFIX, host,
             "/", path, (search) ? "?" : "", (search) ? search : "", NULL);
diff --git a/modules/proxy/mod_proxy_fcgi.c b/modules/proxy/mod_proxy_fcgi.c
index a422b4e20c..831bd15ae9 100644
--- a/modules/proxy/mod_proxy_fcgi.c
+++ b/modules/proxy/mod_proxy_fcgi.c
@@ -102,9 +102,20 @@ static int proxy_fcgi_canon(request_rec *r, char *url)
 
         path = ap_proxy_canonenc_ex(r->pool, url, strlen(url), enc_path, flags,
                                     r->proxyreq);
+        if (!path) {
+            return HTTP_BAD_REQUEST;
+        }
+    }
+    /*
+     * If we have a raw control character or a ' ' in nocanon path,
+     * correct encoding was missed.
+     */
+    if (path == url && *ap_scan_vchar_obstext(path)) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10414)
+                      "To be forwarded path contains control "
+                      "characters or spaces");
+        return HTTP_FORBIDDEN;
     }
-    if (path == NULL)
-        return HTTP_BAD_REQUEST;
 
     r->filename = apr_pstrcat(r->pool, "proxy:fcgi://", host, sport, "/",
                               path, NULL);
diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c
index 657f098069..1842c49e02 100644
--- a/modules/proxy/mod_proxy_http.c
+++ b/modules/proxy/mod_proxy_http.c
@@ -131,26 +131,32 @@ static int proxy_http_canon(request_rec *r, char *url)
 
             path = ap_proxy_canonenc_ex(r->pool, url, strlen(url), enc_path,
                                         flags, r->proxyreq);
+            if (!path) {
+                return HTTP_BAD_REQUEST;
+            }
             search = r->args;
         }
-        if (search && *ap_scan_vchar_obstext(search)) {
-            /*
-             * We have a raw control character or a ' ' in r->args.
-             * Correct encoding was missed.
-             */
-            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10408)
-                          "To be forwarded query string contains control "
-                          "characters or spaces");
-            return HTTP_FORBIDDEN;
-        }
         break;
     case PROXYREQ_PROXY:
         path = url;
         break;
     }
-
-    if (path == NULL)
-        return HTTP_BAD_REQUEST;
+    /*
+     * If we have a raw control character or a ' ' in nocanon path or
+     * r->args, correct encoding was missed.
+     */
+    if (path == url && *ap_scan_vchar_obstext(path)) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10415)
+                      "To be forwarded path contains control "
+                      "characters or spaces");
+        return HTTP_FORBIDDEN;
+    }
+    if (search && *ap_scan_vchar_obstext(search)) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10408)
+                      "To be forwarded query string contains control "
+                      "characters or spaces");
+        return HTTP_FORBIDDEN;
+    }
 
     if (port != def_port)
         apr_snprintf(sport, sizeof(sport), ":%d", port);
diff --git a/modules/proxy/mod_proxy_uwsgi.c b/modules/proxy/mod_proxy_uwsgi.c
index 3bb1f62bea..fd76c95508 100644
--- a/modules/proxy/mod_proxy_uwsgi.c
+++ b/modules/proxy/mod_proxy_uwsgi.c
@@ -94,9 +94,19 @@ static int uwsgi_canon(request_rec *r, char *url)
 
         path = ap_proxy_canonenc_ex(r->pool, url, strlen(url), enc_path, flags,
                                     r->proxyreq);
+        if (!path) {
+            return HTTP_BAD_REQUEST;
+        }
     }
-    if (!path) {
-        return HTTP_BAD_REQUEST;
+    /*
+     * If we have a raw control character or a ' ' in nocanon path,
+     * correct encoding was missed.
+     */
+    if (path == url && *ap_scan_vchar_obstext(path)) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10417)
+                      "To be forwarded path contains control "
+                      "characters or spaces");
+        return HTTP_FORBIDDEN;
     }
 
     r->filename =
diff --git a/modules/proxy/mod_proxy_wstunnel.c b/modules/proxy/mod_proxy_wstunnel.c
index 012dd0a772..30ba1b49ff 100644
--- a/modules/proxy/mod_proxy_wstunnel.c
+++ b/modules/proxy/mod_proxy_wstunnel.c
@@ -120,20 +120,27 @@ static int proxy_wstunnel_canon(request_rec *r, char *url)
 
         path = ap_proxy_canonenc_ex(r->pool, url, strlen(url), enc_path, flags,
                                     r->proxyreq);
+        if (!path) {
+            return HTTP_BAD_REQUEST;
+        }
         search = r->args;
     }
+    /*
+     * If we have a raw control character or a ' ' in nocanon path or
+     * r->args, correct encoding was missed.
+     */
+    if (path == url && *ap_scan_vchar_obstext(path)) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10419)
+                      "To be forwarded path contains control "
+                      "characters or spaces");
+        return HTTP_FORBIDDEN;
+    }
     if (search && *ap_scan_vchar_obstext(search)) {
-        /*
-         * We have a raw control character or a ' ' in r->args.
-         * Correct encoding was missed.
-         */
         ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10409)
                       "To be forwarded query string contains control "
                       "characters or spaces");
         return HTTP_FORBIDDEN;
     }
-    if (path == NULL)
-        return HTTP_BAD_REQUEST;
 
     if (port != def_port)
         apr_snprintf(sport, sizeof(sport), ":%d", port);