*) core: make ap_escape_quotes() work correctly on strings

   with more than MAX_INT/2 characters, counting quotes double.
   Credit to <generalbugs@zippenhop.com> for finding this.

*) core: improved checks in ap_escape_quotes() for
   extra long strings (or resulting strings) that
   exceed ptrdiff_t ranges.

Merge r1899609, r1899905 from trunk.
Reviewed by: icing, rpluem, ylavic
Submitted by: icing, ylavic

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

diff --git a/CHANGES b/CHANGES
index e1bc5d8ecbdb27938709e822e544a7338132399e..bd07e238297d1d0eae0d1ad3a9cb247b9b932b3a 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,11 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.4.54
 
+  *) core: make ap_escape_quotes() work correctly on strings
+     with more than MAX_INT/2 characters, counting quotes double.
+     Credit to <generalbugs@zippenhop.com> for finding this.
+     [Stefan Eissing]
+
   *) mod_md: the `MDCertificateAuthority` directive can take more than one URL/name of
      an ACME CA. This gives a failover for renewals when several consecutive attempts
      to get a certificate failed.

diff --git a/server/util.c b/server/util.c
index 604be1a1ce3af72e3d7e500d1844bc41457620b7..38080dc5dd151a9d2146e8fd6c8e5473cfa5048b 100644 (file)
--- a/server/util.c
+++ b/server/util.c
@@ -2535,7 +2535,7 @@ AP_DECLARE(void) ap_content_type_tolower(char *str)
  */
 AP_DECLARE(char *) ap_escape_quotes(apr_pool_t *p, const char *instring)
 {
-    int newlen = 0;
+    apr_size_t size, extra = 0;
     const char *inchr = instring;
     char *outchr, *outstring;
 
@@ -2544,9 +2544,8 @@ AP_DECLARE(char *) ap_escape_quotes(apr_pool_t *p, const char *instring)
      * string up by an extra byte each time we find an unescaped ".
      */
     while (*inchr != '\0') {
-        newlen++;
         if (*inchr == '"') {
-            newlen++;
+            extra++;
         }
         /*
          * If we find a slosh, and it's not the last byte in the string,
@@ -2554,11 +2553,32 @@ AP_DECLARE(char *) ap_escape_quotes(apr_pool_t *p, const char *instring)
          */
         else if ((*inchr == '\\') && (inchr[1] != '\0')) {
             inchr++;
-            newlen++;
         }
         inchr++;
     }
-    outstring = apr_palloc(p, newlen + 1);
+
+    if (!extra) {
+        return apr_pstrdup(p, instring);
+    }
+
+    /* How large will the string become, once we escaped all the quotes?
+     * The tricky cases are
+     * - an `instring` that is already longer than `ptrdiff_t`
+     *   can hold (which is an undefined case in C, as C defines ptrdiff_t as
+     *   a signed difference between pointers into the same array and one index
+     *   beyond).
+     * - an `instring` that, including the `extra` chars we want to add, becomes
+     *   even larger than apr_size_t can handle.
+     * Since this function was not designed to ever return NULL for failure, we
+     * can only trigger a hard assertion failure. It seems more a programming
+     * mistake (or failure to verify the input causing this) that leads to this
+     * situation.
+     */
+    ap_assert(inchr - instring > 0);
+    size = ((apr_size_t)(inchr - instring)) + 1;
+    ap_assert(size + extra > size);
+
+    outstring = apr_palloc(p, size + extra);
     inchr = instring;
     outchr = outstring;
     /*