diff --git a/docs/log-message-tags/next-number b/docs/log-message-tags/next-number index d067f07618..4319db8166 100644 --- a/docs/log-message-tags/next-number +++ b/docs/log-message-tags/next-number @@ -1 +1 @@ -10635 +10636 diff --git a/modules/aaa/mod_auth_digest.c b/modules/aaa/mod_auth_digest.c index 91faf3de83..29bb8f83e5 100644 --- a/modules/aaa/mod_auth_digest.c +++ b/modules/aaa/mod_auth_digest.c @@ -1515,8 +1515,10 @@ static const char *new_digest(const request_rec *r, NULL)); } -static void copy_uri_components(apr_uri_t *dst, - apr_uri_t *src, request_rec *r) { +static int copy_uri_components(apr_uri_t *dst, + apr_uri_t *src, request_rec *r) { + int rv; + if (src->scheme && src->scheme[0] != '\0') { dst->scheme = src->scheme; } @@ -1526,7 +1528,9 @@ static void copy_uri_components(apr_uri_t *dst, if (src->hostname && src->hostname[0] != '\0') { dst->hostname = apr_pstrdup(r->pool, src->hostname); - ap_unescape_url(dst->hostname); + if ((rv = ap_unescape_url(dst->hostname)) != OK) { + return rv; + } } else { dst->hostname = (char *) ap_get_server_name(r); @@ -1541,7 +1545,9 @@ static void copy_uri_components(apr_uri_t *dst, if (src->path && src->path[0] != '\0') { dst->path = apr_pstrdup(r->pool, src->path); - ap_unescape_url(dst->path); + if ((rv = ap_unescape_url(dst->path)) != OK) { + return rv; + } } else { dst->path = src->path; @@ -1549,13 +1555,16 @@ static void copy_uri_components(apr_uri_t *dst, if (src->query && src->query[0] != '\0') { dst->query = apr_pstrdup(r->pool, src->query); - ap_unescape_url(dst->query); + if ((rv = ap_unescape_url(dst->query)) != OK) { + return rv; + } } else { dst->query = src->query; } dst->hostinfo = src->hostinfo; + return OK; } /* These functions return 0 if client is OK, and proper error status @@ -1631,25 +1640,21 @@ static int authenticate_digest_user(request_rec *r) */ apr_uri_t r_uri, d_uri; - copy_uri_components(&r_uri, resp->psd_request_uri, r); - if (apr_uri_parse(r->pool, resp->uri, &d_uri) != APR_SUCCESS) { + if (copy_uri_components(&r_uri, resp->psd_request_uri, r) != OK) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10635) + "invalid request-uri <%s>", r->unparsed_uri); + return HTTP_BAD_REQUEST; + } + if (apr_uri_parse(r->pool, resp->uri, &d_uri) != APR_SUCCESS + || (d_uri.hostname && ap_unescape_url(d_uri.hostname) != OK) + || (d_uri.path && ap_unescape_url(d_uri.path) != OK) + || (d_uri.query && ap_unescape_url(d_uri.query) != OK)) { ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01783) "invalid uri <%s> in Authorization header", resp->uri); return HTTP_BAD_REQUEST; } - if (d_uri.hostname) { - ap_unescape_url(d_uri.hostname); - } - if (d_uri.path) { - ap_unescape_url(d_uri.path); - } - - if (d_uri.query) { - ap_unescape_url(d_uri.query); - } - if (r->method_number == M_CONNECT) { if (!r_uri.hostinfo || strcmp(resp->uri, r_uri.hostinfo)) { ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01785) diff --git a/test/modules/aaa/test_001_challenge_response.py b/test/modules/aaa/test_001_challenge_response.py index 71080dab07..92b54b5c45 100644 --- a/test/modules/aaa/test_001_challenge_response.py +++ b/test/modules/aaa/test_001_challenge_response.py @@ -178,3 +178,20 @@ class TestDigestChallengeResponse: r = env.curl_get(self.url(env), options=["-H", f"Authorization: {auth}"]) assert r.response["status"] == 401 env.httpd_error_log.ignore_recent(lognos=["AH01794"]) + + def test_digest_015_uri_bad_escaping_rejected(self, env): + # The Authorization uri= is compared against the request-target after + # %-decoding both. A uri= that fails to decode -- an encoded NUL or + # slash -- must be rejected, not silently truncated at the bad octet + # so that its prefix matches the request-target. + challenge = self.challenge(env) + for bad_uri in ["/digest/default/secret.txt%00ignored", + "/digest/default%2fsecret.txt"]: + auth = dc.build_authorization( + AAATestEnv.DIGEST_USER, challenge, AAATestEnv.DIGEST_PASSWORD, + method="GET", uri=bad_uri) + r = env.curl_get(self.url(env), + options=["-H", f"Authorization: {auth}"]) + assert r.response["status"] == 400, \ + f"uri={bad_uri!r} was not rejected" + env.httpd_error_log.ignore_recent(lognos=["AH01783"])