You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ji...@apache.org on 2012/11/26 18:18:57 UTC
svn commit: r1413732 - in /httpd/httpd/trunk/modules: generators/mod_info.c
generators/mod_status.c mappers/mod_imagemap.c proxy/mod_proxy_balancer.c
proxy/mod_proxy_ftp.c
Author: jim
Date: Mon Nov 26 17:18:54 2012
New Revision: 1413732
URL: http://svn.apache.org/viewvc?rev=1413732&view=rev
Log:
Be sure to escape potential troubled srings
Modified:
httpd/httpd/trunk/modules/generators/mod_info.c
httpd/httpd/trunk/modules/generators/mod_status.c
httpd/httpd/trunk/modules/mappers/mod_imagemap.c
httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.c
Modified: httpd/httpd/trunk/modules/generators/mod_info.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/generators/mod_info.c?rev=1413732&r1=1413731&r2=1413732&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/generators/mod_info.c (original)
+++ httpd/httpd/trunk/modules/generators/mod_info.c Mon Nov 26 17:18:54 2012
@@ -459,7 +459,8 @@ static int show_server_settings(request_
MODULE_MAGIC_NUMBER_MINOR);
ap_rprintf(r,
"<dt><strong>Hostname/port:</strong> "
- "<tt>%s:%u</tt></dt>\n", ap_get_server_name(r),
+ "<tt>%s:%u</tt></dt>\n",
+ ap_escape_html(r->pool, ap_get_server_name(r)),
ap_get_server_port(r));
ap_rprintf(r,
"<dt><strong>Timeouts:</strong> "
Modified: httpd/httpd/trunk/modules/generators/mod_status.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/generators/mod_status.c?rev=1413732&r1=1413731&r2=1413732&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/generators/mod_status.c (original)
+++ httpd/httpd/trunk/modules/generators/mod_status.c Mon Nov 26 17:18:54 2012
@@ -400,7 +400,8 @@ static int status_handler(request_rec *r
"<title>Apache Status</title>\n"
"</head><body>\n"
"<h1>Apache Server Status for ", r);
- ap_rvputs(r, ap_get_server_name(r), " (via ", r->connection->local_ip,
+ ap_rvputs(r, ap_escape_html(r->pool, ap_get_server_name(r)),
+ " (via ", r->connection->local_ip,
")</h1>\n\n", NULL);
ap_rvputs(r, "<dl><dt>Server Version: ",
ap_get_server_description(), "</dt>\n", NULL);
Modified: httpd/httpd/trunk/modules/mappers/mod_imagemap.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_imagemap.c?rev=1413732&r1=1413731&r2=1413732&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/mappers/mod_imagemap.c (original)
+++ httpd/httpd/trunk/modules/mappers/mod_imagemap.c Mon Nov 26 17:18:54 2012
@@ -338,7 +338,7 @@ static char *imap_url(request_rec *r, co
if (!strcasecmp(value, "referer")) {
referer = apr_table_get(r->headers_in, "Referer");
if (referer && *referer) {
- return ap_escape_html(r->pool, referer);
+ return referer;
}
else {
/* XXX: This used to do *value = '\0'; ... which is totally bogus
@@ -521,40 +521,50 @@ static void menu_comment(request_rec *r,
static void menu_default(request_rec *r, char *menu, char *href, char *text)
{
+ char *ehref, *etext;
if (!strcasecmp(href, "error") || !strcasecmp(href, "nocontent")) {
return; /* don't print such lines, these aren't
really href's */
}
+
+ ehref = ap_escape_uri(r->pool, href);
+ etext = ap_escape_html(r->pool, text);
+
if (!strcasecmp(menu, "formatted")) {
- ap_rvputs(r, "<pre>(Default) <a href=\"", href, "\">", text,
- "</a></pre>\n", NULL);
+ ap_rvputs(r, "<pre>(Default) <a href=\"", ehref, "\">", etext,
+ "</a></pre>\n", NULL);
}
else if (!strcasecmp(menu, "semiformatted")) {
- ap_rvputs(r, "<pre>(Default) <a href=\"", href, "\">", text,
+ ap_rvputs(r, "<pre>(Default) <a href=\"", ehref, "\">", etext,
"</a></pre>\n", NULL);
}
else if (!strcasecmp(menu, "unformatted")) {
- ap_rvputs(r, "<a href=\"", href, "\">", text, "</a>", NULL);
+ ap_rvputs(r, "<a href=\"", ehref, "\">", etext, "</a>", NULL);
}
return;
}
static void menu_directive(request_rec *r, char *menu, char *href, char *text)
{
+ char *ehref, *etext;
if (!strcasecmp(href, "error") || !strcasecmp(href, "nocontent")) {
return; /* don't print such lines, as this isn't
really an href */
}
+
+ ehref = ap_escape_uri(r->pool, href);
+ etext = ap_escape_html(r->pool, text);
+
if (!strcasecmp(menu, "formatted")) {
- ap_rvputs(r, "<pre> <a href=\"", href, "\">", text,
+ ap_rvputs(r, "<pre> <a href=\"", ehref, "\">", etext,
"</a></pre>\n", NULL);
}
else if (!strcasecmp(menu, "semiformatted")) {
- ap_rvputs(r, "<pre> <a href=\"", href, "\">", text,
+ ap_rvputs(r, "<pre> <a href=\"", ehref, "\">", etext,
"</a></pre>\n", NULL);
}
else if (!strcasecmp(menu, "unformatted")) {
- ap_rvputs(r, "<a href=\"", href, "\">", text, "</a>", NULL);
+ ap_rvputs(r, "<a href=\"", ehref, "\">", etext, "</a>", NULL);
}
return;
}
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1413732&r1=1413731&r2=1413732&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Mon Nov 26 17:18:54 2012
@@ -1428,7 +1428,8 @@ static int balancer_handler(request_rec
"}\n"
"</style>\n</head>\n", r);
ap_rputs("<body><h1>Load Balancer Manager for ", r);
- ap_rvputs(r, ap_get_server_name(r), "</h1>\n\n", NULL);
+ ap_rvputs(r, ap_escape_html(r->pool, ap_get_server_name(r)),
+ "</h1>\n\n", NULL);
ap_rvputs(r, "<dl><dt>Server Version: ",
ap_get_server_description(), "</dt>\n", NULL);
ap_rvputs(r, "<dt>Server Built: ",
@@ -1437,10 +1438,10 @@ static int balancer_handler(request_rec
for (i = 0; i < conf->balancers->nelts; i++) {
ap_rputs("<hr />\n<h3>LoadBalancer Status for ", r);
- ap_rvputs(r, "<a href='", r->uri, "?b=",
+ ap_rvputs(r, "<a href=\"", ap_escape_uri(r->pool, r->uri), "?b=",
balancer->s->name + sizeof(BALANCER_PREFIX) - 1,
"&nonce=", balancer->s->nonce,
- "'>", NULL);
+ "\">", NULL);
ap_rvputs(r, balancer->s->name, "</a> [",balancer->s->sname, "]</h3>\n", NULL);
ap_rputs("\n\n<table><tr>"
"<th>MaxMembers</th><th>StickySession</th><th>DisableFailover</th><th>Timeout</th><th>FailoverAttempts</th><th>Method</th>"
@@ -1487,11 +1488,12 @@ static int balancer_handler(request_rec
for (n = 0; n < balancer->workers->nelts; n++) {
char fbuf[50];
worker = *workers;
- ap_rvputs(r, "<tr>\n<td><a href='", r->uri, "?b=",
+ ap_rvputs(r, "<tr>\n<td><a href=\"",
+ ap_escape_uri(r->pool, r->uri), "?b=",
balancer->s->name + sizeof(BALANCER_PREFIX) - 1, "&w=",
ap_escape_uri(r->pool, worker->s->name),
"&nonce=", balancer->s->nonce,
- "'>", NULL);
+ "\">", NULL);
ap_rvputs(r, worker->s->name, "</a></td>", NULL);
ap_rvputs(r, "<td>", ap_escape_html(r->pool, worker->s->route),
NULL);
@@ -1518,20 +1520,20 @@ static int balancer_handler(request_rec
if (wsel && bsel) {
ap_rputs("<h3>Edit worker settings for ", r);
ap_rvputs(r, wsel->s->name, "</h3>\n", NULL);
- ap_rputs("<form method='POST' enctype='application/x-www-form-urlencoded' action='", r);
- ap_rvputs(r, action, "'>\n", NULL);
+ ap_rputs("<form method=\"POST\" enctype=\"application/x-www-form-urlencoded\" action=\"", r);
+ ap_rvputs(r, ap_escape_uri(r->pool, action), "\">\n", NULL);
ap_rputs("<dl>\n<table><tr><td>Load factor:</td><td><input name='w_lf' id='w_lf' type=text ", r);
ap_rprintf(r, "value='%d'></td></tr>\n", wsel->s->lbfactor);
ap_rputs("<tr><td>LB Set:</td><td><input name='w_ls' id='w_ls' type=text ", r);
ap_rprintf(r, "value='%d'></td></tr>\n", wsel->s->lbset);
ap_rputs("<tr><td>Route:</td><td><input name='w_wr' id='w_wr' type=text ", r);
- ap_rvputs(r, "value='", ap_escape_html(r->pool, wsel->s->route),
+ ap_rvputs(r, "value=\"", ap_escape_html(r->pool, wsel->s->route),
NULL);
- ap_rputs("'></td></tr>\n", r);
+ ap_rputs("\"></td></tr>\n", r);
ap_rputs("<tr><td>Route Redirect:</td><td><input name='w_rr' id='w_rr' type=text ", r);
- ap_rvputs(r, "value='", ap_escape_html(r->pool, wsel->s->redirect),
+ ap_rvputs(r, "value=\"", ap_escape_html(r->pool, wsel->s->redirect),
NULL);
- ap_rputs("'></td></tr>\n", r);
+ ap_rputs("\"></td></tr>\n", r);
ap_rputs("<tr><td>Status:</td>", r);
ap_rputs("<td><table><tr><th>Ign</th><th>Drn</th><th>Dis</th><th>Stby</th></tr>\n<tr>", r);
create_radio("w_status_I", (PROXY_WORKER_IGNORE_ERRORS & wsel->s->status), r);
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.c?rev=1413732&r1=1413731&r2=1413732&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.c Mon Nov 26 17:18:54 2012
@@ -530,7 +530,9 @@ static apr_status_t proxy_send_dir_filte
" </head>\n"
" <body>\n <h2>Directory of "
"<a href=\"/\">%s</a>/%s",
- site, basedir, escpath, site, basedir, escpath, site, str);
+ ap_escape_html(p, site), basedir, escpath,
+ ap_escape_uri(p, site), basedir, escpath,
+ ap_escape_uri(p, site), str);
APR_BRIGADE_INSERT_TAIL(out, apr_bucket_pool_create(str, strlen(str),
p, c->bucket_alloc));
Re: svn commit: r1413732 - in /httpd/httpd/trunk/modules: generators/mod_info.c generators/mod_status.c mappers/mod_imagemap.c proxy/mod_proxy_balancer.c proxy/mod_proxy_ftp.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
It's to avoid double escaping...
On Nov 26, 2012, at 1:38 PM, Nick Kew <ni...@webthing.com> wrote:
> On Mon, 26 Nov 2012 17:18:57 -0000
> jim@apache.org wrote:
>
>
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/mappers/mod_imagemap.c (original)
>> +++ httpd/httpd/trunk/modules/mappers/mod_imagemap.c Mon Nov 26 17:18:54 2012
>> @@ -338,7 +338,7 @@ static char *imap_url(request_rec *r, co
>> if (!strcasecmp(value, "referer")) {
>> referer = apr_table_get(r->headers_in, "Referer");
>> if (referer && *referer) {
>> - return ap_escape_html(r->pool, referer);
>> + return referer;
>> }
>
> Isn't this the opposite change to the others? And a case that looks
> at first glance to be potentially exploitable from a third-party site?
>
>
> --
> Nick Kew
>
Re: svn commit: r1413732 - in /httpd/httpd/trunk/modules:
generators/mod_info.c generators/mod_status.c mappers/mod_imagemap.c
proxy/mod_proxy_balancer.c proxy/mod_proxy_ftp.c
Posted by Nick Kew <ni...@webthing.com>.
On Mon, 26 Nov 2012 17:18:57 -0000
jim@apache.org wrote:
> ==============================================================================
> --- httpd/httpd/trunk/modules/mappers/mod_imagemap.c (original)
> +++ httpd/httpd/trunk/modules/mappers/mod_imagemap.c Mon Nov 26 17:18:54 2012
> @@ -338,7 +338,7 @@ static char *imap_url(request_rec *r, co
> if (!strcasecmp(value, "referer")) {
> referer = apr_table_get(r->headers_in, "Referer");
> if (referer && *referer) {
> - return ap_escape_html(r->pool, referer);
> + return referer;
> }
Isn't this the opposite change to the others? And a case that looks
at first glance to be potentially exploitable from a third-party site?
--
Nick Kew