From 3866a3b5e1043eee91b4ac00dbf14ad90eed56f2 Mon Sep 17 00:00:00 2001 From: Yann Ylavic Date: Thu, 5 Jul 2018 19:20:20 +0000 Subject: [PATCH] Merge r1832280 from trunk: In 'ap_proxy_cookie_reverse_map', iterate over each token of the 'Set-Cookie' header field in order to avoid updating the wrong one. This could happen if the header field has something like 'fakepath=foo;path=bar". In this case fakepath would be updated instead of path. We don't need regex anymore in order to parse the field values and 'ap_proxy_strmatch_domain' and 'ap_proxy_strmatch_path' are now useless. (and should be axed IMHO) PR 61560 Submitted by: jailletc36 Reviewed by: jailletc36, rpluem, ylavic git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1835171 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 5 ++ modules/proxy/proxy_util.c | 102 ++++++++++++++++++++----------------- 2 files changed, 59 insertions(+), 48 deletions(-) diff --git a/CHANGES b/CHANGES index 2fdd62f4e8..214875d76f 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,11 @@ -*- coding: utf-8 -*- Changes with Apache 2.4.34 + *) mod_proxy: Fix a corner case where the ProxyPassReverseCookieDomain or + ProxyPassReverseCookiePath directive could fail to update correctly + 'domain=' or 'path=' in the 'Set-Cookie' header. PR 61560. + [Christophe Jaillet] + *) mod_ratelimit: fix behavior when proxing content. PR 62362. [Luca Toscano, Yann Ylavic] diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c index 9afa1d84d6..7b76144ba7 100644 --- a/modules/proxy/proxy_util.c +++ b/modules/proxy/proxy_util.c @@ -959,7 +959,7 @@ PROXY_DECLARE(const char *) ap_proxy_cookie_reverse_map(request_rec *r, int i; int ddiff = 0; int pdiff = 0; - char *ret; + char *tmpstr, *tmpstr_orig, *token, *last, *ret; if (r->proxyreq != PROXYREQ_REVERSE) { return str; @@ -969,48 +969,56 @@ PROXY_DECLARE(const char *) ap_proxy_cookie_reverse_map(request_rec *r, * Find the match and replacement, but save replacing until we've done * both path and domain so we know the new strlen */ - if ((pathp = apr_strmatch(ap_proxy_strmatch_path, str, len)) != NULL) { - pathp += 5; - poffs = pathp - str; - pathe = ap_strchr_c(pathp, ';'); - l1 = pathe ? (pathe - pathp) : strlen(pathp); - pathe = pathp + l1 ; - if (conf->interpolate_env == 1) { - ent = (struct proxy_alias *)rconf->cookie_paths->elts; + tmpstr_orig = tmpstr = apr_pstrdup(r->pool, str); + while ((token = apr_strtok(tmpstr, ";", &last))) { + /* skip leading spaces */ + while (apr_isspace(*token)) { + ++token; } - else { - ent = (struct proxy_alias *)conf->cookie_paths->elts; - } - for (i = 0; i < conf->cookie_paths->nelts; i++) { - l2 = strlen(ent[i].fake); - if (l1 >= l2 && strncmp(ent[i].fake, pathp, l2) == 0) { - newpath = ent[i].real; - pdiff = strlen(newpath) - l1; - break; - } - } - } - if ((domainp = apr_strmatch(ap_proxy_strmatch_domain, str, len)) != NULL) { - domainp += 7; - doffs = domainp - str; - domaine = ap_strchr_c(domainp, ';'); - l1 = domaine ? (domaine - domainp) : strlen(domainp); - domaine = domainp + l1; - if (conf->interpolate_env == 1) { - ent = (struct proxy_alias *)rconf->cookie_domains->elts; - } - else { - ent = (struct proxy_alias *)conf->cookie_domains->elts; + if (ap_cstr_casecmpn("path=", token, 5) == 0) { + pathp = token + 5; + poffs = pathp - tmpstr_orig; + l1 = strlen(pathp); + pathe = str + poffs + l1; + if (conf->interpolate_env == 1) { + ent = (struct proxy_alias *)rconf->cookie_paths->elts; + } + else { + ent = (struct proxy_alias *)conf->cookie_paths->elts; + } + for (i = 0; i < conf->cookie_paths->nelts; i++) { + l2 = strlen(ent[i].fake); + if (l1 >= l2 && strncmp(ent[i].fake, pathp, l2) == 0) { + newpath = ent[i].real; + pdiff = strlen(newpath) - l1; + break; + } + } } - for (i = 0; i < conf->cookie_domains->nelts; i++) { - l2 = strlen(ent[i].fake); - if (l1 >= l2 && strncasecmp(ent[i].fake, domainp, l2) == 0) { - newdomain = ent[i].real; - ddiff = strlen(newdomain) - l1; - break; + else if (ap_cstr_casecmpn("domain=", token, 7) == 0) { + domainp = token + 7; + doffs = domainp - tmpstr_orig; + l1 = strlen(domainp); + domaine = str + doffs + l1; + if (conf->interpolate_env == 1) { + ent = (struct proxy_alias *)rconf->cookie_domains->elts; + } + else { + ent = (struct proxy_alias *)conf->cookie_domains->elts; + } + for (i = 0; i < conf->cookie_domains->nelts; i++) { + l2 = strlen(ent[i].fake); + if (l1 >= l2 && strncasecmp(ent[i].fake, domainp, l2) == 0) { + newdomain = ent[i].real; + ddiff = strlen(newdomain) - l1; + break; + } } } + + /* Iterate the remaining tokens using apr_strtok(NULL, ...) */ + tmpstr = NULL; } if (newpath) { @@ -1021,14 +1029,14 @@ PROXY_DECLARE(const char *) ap_proxy_cookie_reverse_map(request_rec *r, if (doffs > poffs) { memcpy(ret, str, poffs); memcpy(ret + poffs, newpath, l1); - memcpy(ret + poffs + l1, pathe, domainp - pathe); + memcpy(ret + poffs + l1, pathe, str + doffs - pathe); memcpy(ret + doffs + pdiff, newdomain, l2); strcpy(ret + doffs + pdiff + l2, domaine); } else { memcpy(ret, str, doffs) ; memcpy(ret + doffs, newdomain, l2); - memcpy(ret + doffs + l2, domaine, pathp - domaine); + memcpy(ret + doffs + l2, domaine, str + poffs - domaine); memcpy(ret + poffs + ddiff, newpath, l1); strcpy(ret + poffs + ddiff + l1, pathe); } @@ -1039,17 +1047,15 @@ PROXY_DECLARE(const char *) ap_proxy_cookie_reverse_map(request_rec *r, strcpy(ret + poffs + l1, pathe); } } - else { - if (newdomain) { - ret = apr_palloc(r->pool, len + pdiff + ddiff + 1); + else if (newdomain) { + ret = apr_palloc(r->pool, len + ddiff + 1); l2 = strlen(newdomain); memcpy(ret, str, doffs); memcpy(ret + doffs, newdomain, l2); - strcpy(ret + doffs+l2, domaine); - } - else { - ret = (char *)str; /* no change */ - } + strcpy(ret + doffs + l2, domaine); + } + else { + ret = (char *)str; /* no change */ } return ret; -- 2.40.0