Merge r1893971 from trunk:

core: Add ap_unescape_url_ex() for better decoding control, and deprecate
      unused AP_NORMALIZE_DROP_PARAMETERS flag.

Submitted by: ylavic
Reviewed by: ylavic, icing, gbechis

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

diff --git a/CHANGES b/CHANGES
index 4bab8495096a6a4786baf9e13bbf3867b41c73bf..093d46f99a6beb82ae21c861535e4bc9b14476ed 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.4.51
 
+  *) core: Add ap_unescape_url_ex() for better decoding control, and deprecate
+     unused AP_NORMALIZE_DROP_PARAMETERS flag.
+     [Yann Ylavic, Ruediger Pluem, Stefan Eissing, Joe Orton]
+
 Changes with Apache 2.4.50
 
   *) SECURITY: CVE-2021-41773: Path traversal and file disclosure

diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index 52876f843f27e43ff9cdfff250e6ab44ad476ce5..b6dbd346405b9652e046d5435015ecac24082ec5 100644 (file)
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -579,6 +579,9 @@
  *                           ap_proxy_define_worker_ex() to mod_proxy.h
  * 20120211.116 (2.4.49-dev) add conn_rec->outgoing and ap_ssl_bind_outgoing()
  * 20120211.117 (2.4.50-dev) Add ap_pre_connection
+ * 20210926.1 (2.5.1-dev)  Add ap_unescape_url_ex() and deprecate
+ *                         AP_NORMALIZE_DROP_PARAMETERS
+ * 
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */

diff --git a/include/httpd.h b/include/httpd.h
index d03626a62b52c2574070dbc384d5866d161477d3..2057ec31b2c4fdc4063d6b81204c7c23c2e8deda 100644 (file)
--- a/include/httpd.h
+++ b/include/httpd.h
@@ -1741,6 +1741,18 @@ AP_DECLARE(int) ap_unescape_url(char *url);
  */
 AP_DECLARE(int) ap_unescape_url_keep2f(char *url, int decode_slashes);
 
+#define AP_UNESCAPE_URL_KEEP_UNRESERVED (1u << 0)
+#define AP_UNESCAPE_URL_FORBID_SLASHES  (1u << 1)
+#define AP_UNESCAPE_URL_KEEP_SLASHES    (1u << 2)
+
+/**
+ * Unescape a URL, with options
+ * @param url The url to unescape
+ * @param flags Bitmask of AP_UNESCAPE_URL_* flags
+ * @return 0 on success, non-zero otherwise
+ */
+AP_DECLARE(int) ap_unescape_url_ex(char *url, unsigned int flags);
+
 /**
  * Unescape an application/x-www-form-urlencoded string
  * @param query The query to unescape
@@ -1768,7 +1780,7 @@ AP_DECLARE(void) ap_no2slash_ex(char *name, int is_fs_path);
 #define AP_NORMALIZE_NOT_ABOVE_ROOT     (1u <<  1)
 #define AP_NORMALIZE_DECODE_UNRESERVED  (1u <<  2)
 #define AP_NORMALIZE_MERGE_SLASHES      (1u <<  3)
-#define AP_NORMALIZE_DROP_PARAMETERS    (1u <<  4)
+#define AP_NORMALIZE_DROP_PARAMETERS    (0) /* deprecated */
 
 /**
  * Remove all ////, /./ and /xx/../ substrings from a path, and more

diff --git a/server/gen_test_char.c b/server/gen_test_char.c
index 6c6566ec3c06306681b2cc55133e14bd77733f16..248216b03b0a4bc0f732cb030dfdd8f9ff15941b 100644 (file)
--- a/server/gen_test_char.c
+++ b/server/gen_test_char.c
@@ -54,6 +54,7 @@
 #define T_ESCAPE_URLENCODED   (0x40)
 #define T_HTTP_CTRLS          (0x80)
 #define T_VCHAR_OBSTEXT      (0x100)
+#define T_URI_UNRESERVED     (0x200)
 
 int main(int argc, char *argv[])
 {
@@ -71,6 +72,7 @@ int main(int argc, char *argv[])
            "#define T_ESCAPE_URLENCODED    (%u)\n"
            "#define T_HTTP_CTRLS           (%u)\n"
            "#define T_VCHAR_OBSTEXT        (%u)\n"
+           "#define T_URI_UNRESERVED       (%u)\n"
            "\n"
            "static const unsigned short test_char_table[256] = {",
            T_ESCAPE_SHELL_CMD,
@@ -81,7 +83,9 @@ int main(int argc, char *argv[])
            T_ESCAPE_FORENSIC,
            T_ESCAPE_URLENCODED,
            T_HTTP_CTRLS,
-           T_VCHAR_OBSTEXT);
+           T_VCHAR_OBSTEXT,
+           T_URI_UNRESERVED
+        );
 
     for (c = 0; c < 256; ++c) {
         flags = 0;
@@ -164,6 +168,12 @@ int main(int argc, char *argv[])
             flags |= T_ESCAPE_FORENSIC;
         }
 
+        /* Characters in the RFC 3986 "unreserved" set.
+         * https://datatracker.ietf.org/doc/html/rfc3986#section-2.3 */
+        if (c && (apr_isalnum(c) || strchr("-._~", c))) {
+            flags |= T_URI_UNRESERVED;
+        }
+        
         printf("0x%03x%c", flags, (c < 255) ? ',' : ' ');
     }
 

diff --git a/server/request.c b/server/request.c
index eb30f6b478137d87ca000b736450382e10e53cf0..5599b2c01eb5e8bda5d9d86f0e71dd40fd690cc1 100644 (file)
--- a/server/request.c
+++ b/server/request.c
@@ -241,14 +241,15 @@ AP_DECLARE(int) ap_process_request_internal(request_rec *r)
     /* Ignore URL unescaping for translated URIs already */
     if (access_status != DONE && r->parsed_uri.path) {
         core_dir_config *d = ap_get_core_module_config(r->per_dir_config);
-
-        if (d->allow_encoded_slashes) {
-            access_status = ap_unescape_url_keep2f(r->parsed_uri.path,
-                                                   d->decode_encoded_slashes);
+        /* Unreserved chars were already decoded by ap_normalize_path() */
+        unsigned int unescape_flags = AP_UNESCAPE_URL_KEEP_UNRESERVED;
+        if (!d->allow_encoded_slashes) {
+            unescape_flags |= AP_UNESCAPE_URL_FORBID_SLASHES;
         }
-        else {
-            access_status = ap_unescape_url(r->parsed_uri.path);
+        else if (!d->decode_encoded_slashes) {
+            unescape_flags |= AP_UNESCAPE_URL_KEEP_SLASHES;
         }
+        access_status = ap_unescape_url_ex(r->parsed_uri.path, unescape_flags);
         if (access_status) {
             if (access_status == HTTP_NOT_FOUND) {
                 if (! d->allow_encoded_slashes) {

diff --git a/server/util.c b/server/util.c
index 896574b4c57faa2a345563de43eb861d926a8cf4..1dc512b1d4a72f4319f58ade4965e427cca0f2d8 100644 (file)
--- a/server/util.c
+++ b/server/util.c
@@ -530,23 +530,20 @@ AP_DECLARE(int) ap_normalize_path(char *path, unsigned int flags)
          *  be decoded to their corresponding unreserved characters by
          *  URI normalizers.
          */
-        if (decode_unreserved
-                && path[l] == '%' && apr_isxdigit(path[l + 1])
-                                  && apr_isxdigit(path[l + 2])) {
-            const char c = x2c(&path[l + 1]);
-            if (apr_isalnum(c) || (c && strchr("-._~", c))) {
-                /* Replace last char and fall through as the current
-                 * read position */
-                l += 2;
-                path[l] = c;
+        if (decode_unreserved && path[l] == '%') {
+            if (apr_isxdigit(path[l + 1]) && apr_isxdigit(path[l + 2])) {
+                const char c = x2c(&path[l + 1]);
+                if (TEST_CHAR(c, T_URI_UNRESERVED)) {
+                    /* Replace last char and fall through as the current
+                     * read position */
+                    l += 2;
+                    path[l] = c;
+                }
+            }
+            else {
+                /* Invalid encoding */
+                ret = 0;
             }
-        }
-
-        if ((flags & AP_NORMALIZE_DROP_PARAMETERS) && path[l] == ';') {
-            do {
-                l++;
-            } while (!IS_SLASH_OR_NUL(path[l]));
-            continue;
         }
 
         if (w == 0 || IS_SLASH(path[w - 1])) {
@@ -1889,8 +1886,12 @@ static char x2c(const char *what)
  *   decoding %00 or a forbidden character returns HTTP_NOT_FOUND
  */
 
-static int unescape_url(char *url, const char *forbid, const char *reserved)
+static int unescape_url(char *url, const char *forbid, const char *reserved,
+                        unsigned int flags)
 {
+    const int keep_slashes = (flags & AP_UNESCAPE_URL_KEEP_SLASHES) != 0,
+              forbid_slashes = (flags & AP_UNESCAPE_URL_FORBID_SLASHES) != 0,
+              keep_unreserved = (flags & AP_UNESCAPE_URL_KEEP_UNRESERVED) != 0;
     int badesc, badpath;
     char *x, *y;
 
@@ -1915,12 +1916,16 @@ static int unescape_url(char *url, const char *forbid, const char *reserved)
                 char decoded;
                 decoded = x2c(y + 1);
                 if ((decoded == '\0')
+                    || (forbid_slashes && IS_SLASH(decoded))
                     || (forbid && ap_strchr_c(forbid, decoded))) {
                     badpath = 1;
                     *x = decoded;
                     y += 2;
                 }
-                else if (reserved && ap_strchr_c(reserved, decoded)) {
+                else if ((keep_unreserved && TEST_CHAR(decoded,
+                                                       T_URI_UNRESERVED))
+                         || (keep_slashes && IS_SLASH(decoded))
+                         || (reserved && ap_strchr_c(reserved, decoded))) {
                     *x++ = *y++;
                     *x++ = *y++;
                     *x = *y;
@@ -1946,19 +1951,24 @@ static int unescape_url(char *url, const char *forbid, const char *reserved)
 AP_DECLARE(int) ap_unescape_url(char *url)
 {
     /* Traditional */
-    return unescape_url(url, SLASHES, NULL);
+    return unescape_url(url, SLASHES, NULL, 0);
 }
 AP_DECLARE(int) ap_unescape_url_keep2f(char *url, int decode_slashes)
 {
     /* AllowEncodedSlashes (corrected) */
     if (decode_slashes) {
         /* no chars reserved */
-        return unescape_url(url, NULL, NULL);
+        return unescape_url(url, NULL, NULL, 0);
     } else {
         /* reserve (do not decode) encoded slashes */
-        return unescape_url(url, NULL, SLASHES);
+        return unescape_url(url, NULL, SLASHES, 0);
     }
 }
+AP_DECLARE(int) ap_unescape_url_ex(char *url, unsigned int flags)
+{
+    return unescape_url(url, NULL, NULL, flags);
+}
+
 #ifdef NEW_APIS
 /* IFDEF these out until they've been thought through.
  * Just a germ of an API extension for now
@@ -1968,7 +1978,7 @@ AP_DECLARE(int) ap_unescape_url_proxy(char *url)
     /* leave RFC1738 reserved characters intact, * so proxied URLs
      * don't get mangled.  Where does that leave encoded '&' ?
      */
-    return unescape_url(url, NULL, "/;?");
+    return unescape_url(url, NULL, "/;?", 0);
 }
 AP_DECLARE(int) ap_unescape_url_reserved(char *url, const char *reserved)
 {
@@ -1990,7 +2000,7 @@ AP_DECLARE(int) ap_unescape_urlencoded(char *query)
     }
 
     /* unescape everything else */
-    return unescape_url(query, NULL, NULL);
+    return unescape_url(query, NULL, NULL, 0);
 }
 
 AP_DECLARE(char *) ap_construct_server(apr_pool_t *p, const char *hostname,
@@ -2006,7 +2016,7 @@ AP_DECLARE(char *) ap_construct_server(apr_pool_t *p, const char *hostname,
 
 AP_DECLARE(int) ap_unescape_all(char *url)
 {
-    return unescape_url(url, NULL, NULL);
+    return unescape_url(url, NULL, NULL, 0);
 }
 
 /* c2x takes an unsigned, and expects the caller has guaranteed that