From: Yann Ylavic Date: Tue, 24 May 2022 08:55:16 +0000 (+0000) Subject: *) core: make ap_escape_quotes() work correctly on strings X-Git-Tag: 2.4.54-rc1-candidate~51 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=229dc3ac47e0858a0b67227fa878a60f09ee5293;p=thirdparty%2Fapache%2Fhttpd.git *) core: make ap_escape_quotes() work correctly on strings with more than MAX_INT/2 characters, counting quotes double. Credit to 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 e1bc5d8ecbd..bd07e238297 100644 --- 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 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 604be1a1ce3..38080dc5dd1 100644 --- 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; /*