From: Stefan Eissing Date: Fri, 9 Aug 2019 11:59:15 +0000 (+0000) Subject: Merge of r1864693,1864695,1864703 from trunk; X-Git-Tag: 2.4.41~7 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=779adda0d4c7391c3410023e92f7fcb36ac09598;p=thirdparty%2Fapache%2Fhttpd.git Merge of r1864693,1864695,1864703 from trunk; *) mod_proxy: Improve XSRF/XSS protection. [Joe Orton] git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1864787 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index 2021ad2c9ec..887cc257b58 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,8 @@ -*- coding: utf-8 -*- Changes with Apache 2.4.41 + *) mod_proxy: Improve XSRF/XSS protection. [Joe Orton] + *) mod_session: Introduce SessionExpiryUpdateInterval which allows to configure the session/cookie expiry's update interval. PR 57300. [Paul Spangler ] diff --git a/STATUS b/STATUS index 0bc1578c5d1..725ace6de83 100644 --- a/STATUS +++ b/STATUS @@ -150,13 +150,6 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK: reworked, etc.. I would like to avoid a patch that is clearly wrong for some reviewer sitting here for months/years without any action item :) - *) mod_proxy: Improve XSRF/XSS protection. - trunk patch: http://svn.apache.org/r1864693 - http://svn.apache.org/r1864695 - http://svn.apache.org/r1864703 - 2.4.x patch: http://people.apache.org/~jorton/ap_pb_xsrf.patch - +1: jorton, covener, icing - PATCHES/ISSUES THAT ARE BEING WORKED [ New entries should be added at the START of the list ] diff --git a/modules/proxy/mod_proxy_balancer.c b/modules/proxy/mod_proxy_balancer.c index 57756c744bb..398ff4f52c0 100644 --- a/modules/proxy/mod_proxy_balancer.c +++ b/modules/proxy/mod_proxy_balancer.c @@ -1095,6 +1095,18 @@ static void push2table(const char *input, apr_table_t *params, } } +/* Returns non-zero if the Referer: header value passed matches the + * host of the request. */ +static int safe_referer(request_rec *r, const char *ref) +{ + apr_uri_t uri; + + if (apr_uri_parse(r->pool, ref, &uri) || !uri.hostname) + return 0; + + return strcmp(uri.hostname, ap_get_server_name(r)) == 0; +} + /* Manages the loadfactors and member status * The balancer, worker and nonce are obtained from * the request args (?b=...&w=...&nonce=....). @@ -1113,7 +1125,7 @@ static int balancer_handler(request_rec *r) apr_table_t *params; int i, n; int ok2change = 1; - const char *name; + const char *name, *ref; const char *action; apr_status_t rv; @@ -1169,6 +1181,16 @@ static int balancer_handler(request_rec *r) buf[len] = '\0'; push2table(buf, params, NULL, r->pool); } + + /* Ignore parameters if this looks like XSRF */ + ref = apr_table_get(r->headers_in, "Referer"); + if (apr_table_elts(params) + && (!ref || !safe_referer(r, ref))) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10187) + "ignoring params in balancer-manager cross-site access"); + apr_table_clear(params); + } + if ((name = apr_table_get(params, "b"))) bsel = ap_proxy_get_balancer(r->pool, conf, apr_pstrcat(r->pool, BALANCER_PREFIX, name, NULL), 0); @@ -1440,7 +1462,7 @@ static int balancer_handler(request_rec *r) /* Start proxy_balancer */ ap_rvputs(r, " ", balancer->s->name, "\n", NULL); if (*balancer->s->sticky) { - ap_rvputs(r, " ", balancer->s->sticky, + ap_rvputs(r, " ", ap_escape_html(r->pool, balancer->s->sticky), "\n", NULL); ap_rprintf(r, " %s\n", @@ -1650,10 +1672,10 @@ static int balancer_handler(request_rec *r) for (i = 0; i < conf->balancers->nelts; i++) { ap_rputs("
\n

LoadBalancer Status for ", r); - ap_rvputs(r, "", NULL); + "\">", NULL); ap_rvputs(r, balancer->s->name, " [",balancer->s->sname, "]

\n", NULL); ap_rputs("\n\n" "" @@ -1664,11 +1686,11 @@ static int balancer_handler(request_rec *r) balancer->max_workers - (int)storage->num_free_slots(balancer->wslot)); if (*balancer->s->sticky) { if (strcmp(balancer->s->sticky, balancer->s->sticky_path)) { - ap_rvputs(r, "\n", NULL); ap_rvputs(r, "", apr_time_as_msec(worker->s->interval)); ap_rprintf(r, "", worker->s->passes,worker->s->pcount); ap_rprintf(r, "", worker->s->fails, worker->s->fcount); - ap_rprintf(r, "", worker->s->hcuri); + ap_rprintf(r, "", ap_escape_html(r->pool, worker->s->hcuri)); ap_rprintf(r, "\n", r); @@ -1747,20 +1769,20 @@ static int balancer_handler(request_rec *r) if (wsel && bsel) { ap_rputs("

Edit worker settings for ", r); ap_rvputs(r, (*wsel->s->uds_path?"":""), ap_proxy_worker_name(r->pool, wsel), (*wsel->s->uds_path?"":""), "

\n", NULL); - ap_rputs("\n", NULL); + ap_rputs("pool, action), "\">\n", NULL); ap_rputs("
MaxMembersStickySessionDisableFailoverTimeoutFailoverAttemptsMethod", balancer->s->sticky, " | ", - balancer->s->sticky_path, NULL); + ap_rvputs(r, "", ap_escape_html(r->pool, balancer->s->sticky), " | ", + ap_escape_html(r->pool, balancer->s->sticky_path), NULL); } else { - ap_rvputs(r, "", balancer->s->sticky, NULL); + ap_rvputs(r, "", ap_escape_html(r->pool, balancer->s->sticky), NULL); } } else { @@ -1703,12 +1725,12 @@ static int balancer_handler(request_rec *r) for (n = 0; n < balancer->workers->nelts; n++) { char fbuf[50]; worker = *workers; - ap_rvputs(r, "
", NULL); + "\">", NULL); ap_rvputs(r, (*worker->s->uds_path ? "" : ""), ap_proxy_worker_name(r->pool, worker), (*worker->s->uds_path ? "" : ""), "", ap_escape_html(r->pool, worker->s->route), @@ -1730,7 +1752,7 @@ static int balancer_handler(request_rec *r) ap_rprintf(r, "%" APR_TIME_T_FMT "ms%d (%d)%d (%d)%s%s%s", worker->s->hcexpr); } ap_rputs("
\n", (float)(wsel->s->lbfactor)/100.0); ap_rputs("\n", wsel->s->lbset); ap_rputs("\n", r); + ap_rputs("\">\n", r); ap_rputs("\n", r); + ap_rputs("\">\n", r); ap_rputs("", r); ap_rputs("\n", r); } ap_rputs("\n", r); ap_rvputs(r, "
Load factor:
LB Set:
Route:
Route Redirect:
Status:" "" @@ -1805,15 +1827,15 @@ static int balancer_handler(request_rec *r) ap_rprintf(r, "\n", wsel->s->fails); ap_rprintf(r, "\n", ap_escape_html(r->pool, wsel->s->hcuri)); + "value=\"%s\">\n", ap_escape_html(r->pool, wsel->s->hcuri)); ap_rputs("
Ignore Errors
Fails trigger)
HC uri
\n
\n\n", NULL); + ap_rvputs(r, "value=\"", ap_escape_uri(r->pool, wsel->s->name), "\">\n", NULL); ap_rvputs(r, "\n", NULL); + ap_rvputs(r, "value=\"", ap_escape_html(r->pool, bsel->s->name + sizeof(BALANCER_PREFIX) - 1), + "\">\n", NULL); ap_rvputs(r, "\n", NULL); ap_rputs("\n", r); @@ -1823,9 +1845,9 @@ static int balancer_handler(request_rec *r) const ap_list_provider_names_t *pname; int i; ap_rputs("

Edit balancer settings for ", r); - ap_rvputs(r, bsel->s->name, "

\n", NULL); - ap_rputs("
\n", NULL); + ap_rvputs(r, ap_escape_html(r->pool, bsel->s->name), "\n", NULL); + ap_rputs("pool, action), "\">\n", NULL); ap_rputs("\n", r); provs = ap_list_provider_names(r->pool, PROXY_LBMETHOD, "0"); if (provs) { @@ -1849,13 +1871,13 @@ static int balancer_handler(request_rec *r) ap_rputs("\n", r); ap_rputs("\n", r); + ap_rputs("\">    (Use '-' to delete)\n", r); if (storage->num_free_slots(bsel->wslot) != 0) { ap_rputs("\n", r); ap_rvputs(r, "
Sticky Session:s->sticky, bsel->s->sticky_path)) { - ap_rvputs(r, "value ='", bsel->s->sticky, " | ", - bsel->s->sticky_path, NULL); + ap_rvputs(r, "value =\"", ap_escape_html(r->pool, bsel->s->sticky), " | ", + ap_escape_html(r->pool, bsel->s->sticky_path), NULL); } else { - ap_rvputs(r, "value ='", bsel->s->sticky, NULL); + ap_rvputs(r, "value =\"", ap_escape_html(r->pool, bsel->s->sticky), NULL); } - ap_rputs("'>    (Use '-' to delete)
Add New Worker:" "    Are you sure? " @@ -1863,8 +1885,8 @@ static int balancer_handler(request_rec *r) } ap_rputs("
\n\n", NULL); + ap_rvputs(r, "value=\"", ap_escape_html(r->pool, bsel->s->name + sizeof(BALANCER_PREFIX) - 1), + "\">\n", NULL); ap_rvputs(r, "\n", NULL); ap_rputs("
\n", r); diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c index 5482ab8a483..732348b3b82 100644 --- a/modules/proxy/proxy_util.c +++ b/modules/proxy/proxy_util.c @@ -1252,10 +1252,11 @@ PROXY_DECLARE(apr_status_t) ap_proxy_share_balancer(proxy_balancer *balancer, if (*balancer->s->nonce == PROXY_UNSET_NONCE) { char nonce[APR_UUID_FORMATTED_LENGTH + 1]; apr_uuid_t uuid; - /* Retrieve a UUID and store the nonce for the lifetime of - * the process. - */ - apr_uuid_get(&uuid); + + /* Generate a pseudo-UUID from the PRNG to use as a nonce for + * the lifetime of the process. uuid.data is a char array so + * this is an adequate substitute for apr_uuid_get(). */ + ap_random_insecure_bytes(uuid.data, sizeof uuid.data); apr_uuid_format(nonce, &uuid); rv = PROXY_STRNCPY(balancer->s->nonce, nonce); }