Mailing List Archive

Re: svn commit: r1893977 - in /httpd/httpd/branches/2.4.x: ./ CHANGES include/ap_mmn.h include/httpd.h server/gen_test_char.c server/request.c server/util.c
Am 07.10.2021 um 14:27 schrieb ylavic@apache.org:
> Modified: httpd/httpd/branches/2.4.x/include/ap_mmn.h
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/include/ap_mmn.h?rev=1893977&r1=1893976&r2=1893977&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.4.x/include/ap_mmn.h (original)
> +++ httpd/httpd/branches/2.4.x/include/ap_mmn.h Thu Oct 7 12:27:43 2021
> @@ -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" */

Doesn't this need (a cosmetic) adjustment for 2.4.x?

Plus: if a minor bump is needed, this commit contains only a comment change.

Thanks for your intensive work!

Rainer
Re: svn commit: r1893977 - in /httpd/httpd/branches/2.4.x: ./ CHANGES include/ap_mmn.h include/httpd.h server/gen_test_char.c server/request.c server/util.c [ In reply to ]
On 10/7/21 2:27 PM, ylavic@apache.org wrote:
> Author: ylavic
> Date: Thu Oct 7 12:27:43 2021
> New Revision: 1893977
>
> URL: http://svn.apache.org/viewvc?rev=1893977&view=rev
> Log:
> 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
>
> Modified:
> httpd/httpd/branches/2.4.x/ (props changed)
> httpd/httpd/branches/2.4.x/CHANGES
> httpd/httpd/branches/2.4.x/include/ap_mmn.h
> httpd/httpd/branches/2.4.x/include/httpd.h
> httpd/httpd/branches/2.4.x/server/gen_test_char.c
> httpd/httpd/branches/2.4.x/server/request.c
> httpd/httpd/branches/2.4.x/server/util.c
>
> Propchange: httpd/httpd/branches/2.4.x/
> ------------------------------------------------------------------------------
> Merged /httpd/httpd/trunk:r1893971
>
> Modified: httpd/httpd/branches/2.4.x/CHANGES
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/CHANGES?rev=1893977&r1=1893976&r2=1893977&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.4.x/CHANGES [utf-8] (original)
> +++ httpd/httpd/branches/2.4.x/CHANGES [utf-8] Thu Oct 7 12:27:43 2021
> @@ -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
>
> Modified: httpd/httpd/branches/2.4.x/include/ap_mmn.h
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/include/ap_mmn.h?rev=1893977&r1=1893976&r2=1893977&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.4.x/include/ap_mmn.h (original)
> +++ httpd/httpd/branches/2.4.x/include/ap_mmn.h Thu Oct 7 12:27:43 2021
> @@ -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
> + *
> */
>

This is wrong and needs fixing. I have the below fix in my working copy that I can commit instantly:

Index: include/ap_mmn.h
===================================================================
--- include/ap_mmn.h (revision 1893979)
+++ include/ap_mmn.h (working copy)
@@ -579,7 +579,7 @@
* 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
+ * 20120211.118 (2.4.51-dev) Add ap_unescape_url_ex() and deprecate
* AP_NORMALIZE_DROP_PARAMETERS
*
*/
@@ -589,7 +589,7 @@
#ifndef MODULE_MAGIC_NUMBER_MAJOR
#define MODULE_MAGIC_NUMBER_MAJOR 20120211
#endif
-#define MODULE_MAGIC_NUMBER_MINOR 117 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 118 /* 0...n */

/**
* Determine if the server's current MODULE_MAGIC_NUMBER is at least a


Regards

RĂ¼diger


> #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
>
> Modified: httpd/httpd/branches/2.4.x/include/httpd.h
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/include/httpd.h?rev=1893977&r1=1893976&r2=1893977&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.4.x/include/httpd.h (original)
> +++ httpd/httpd/branches/2.4.x/include/httpd.h Thu Oct 7 12:27:43 2021
> @@ -1741,6 +1741,18 @@ AP_DECLARE(int) ap_unescape_url(char *ur
> */
> 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 *na
> #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
>
> Modified: httpd/httpd/branches/2.4.x/server/gen_test_char.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/server/gen_test_char.c?rev=1893977&r1=1893976&r2=1893977&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.4.x/server/gen_test_char.c (original)
> +++ httpd/httpd/branches/2.4.x/server/gen_test_char.c Thu Oct 7 12:27:43 2021
> @@ -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) ? ',' : ' ');
> }
>
>
> Modified: httpd/httpd/branches/2.4.x/server/request.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/server/request.c?rev=1893977&r1=1893976&r2=1893977&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.4.x/server/request.c (original)
> +++ httpd/httpd/branches/2.4.x/server/request.c Thu Oct 7 12:27:43 2021
> @@ -241,14 +241,15 @@ AP_DECLARE(int) ap_process_request_inter
> /* 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) {
>
> Modified: httpd/httpd/branches/2.4.x/server/util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/server/util.c?rev=1893977&r1=1893976&r2=1893977&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.4.x/server/util.c (original)
> +++ httpd/httpd/branches/2.4.x/server/util.c Thu Oct 7 12:27:43 2021
> @@ -530,23 +530,20 @@ AP_DECLARE(int) ap_normalize_path(char *
> * 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 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
> 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(ch
> /* 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(c
> }
>
> /* 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(a
>
> 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
>
>
>
Re: svn commit: r1893977 - in /httpd/httpd/branches/2.4.x: ./ CHANGES include/ap_mmn.h include/httpd.h server/gen_test_char.c server/request.c server/util.c [ In reply to ]
On Thu, Oct 7, 2021 at 2:31 PM Rainer Jung <rainer.jung@kippdata.de> wrote:
>
> Am 07.10.2021 um 14:27 schrieb ylavic@apache.org:
> > Modified: httpd/httpd/branches/2.4.x/include/ap_mmn.h
> > URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/include/ap_mmn.h?rev=1893977&r1=1893976&r2=1893977&view=diff
> > ==============================================================================
> > --- httpd/httpd/branches/2.4.x/include/ap_mmn.h (original)
> > +++ httpd/httpd/branches/2.4.x/include/ap_mmn.h Thu Oct 7 12:27:43 2021
> > @@ -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" */
>
> Doesn't this need (a cosmetic) adjustment for 2.4.x?

Yeah, was wrong in original trunk r1893971 too so I fixed both.

Thanks Rainer (and RĂ¼diger) for noticing.