You are viewing a plain text version of this content. The canonical link for it is here.
Posted to docs@httpd.apache.org by Joshua Slive <jo...@slive.ca> on 2002/08/25 02:47:58 UTC
[Fwd: Re: cvs commit: httpd-2.0/docs/error/include top.html]
I should have copied this to docs@...
-------- Original Message --------
Subject: Re: cvs commit: httpd-2.0/docs/error/include top.html
Date: Sat, 24 Aug 2002 20:03:44 -0400
From: Joshua Slive <jo...@slive.ca>
Reply-To: dev@httpd.apache.org
To: dev@httpd.apache.org
References: <20...@icarus.apache.org>
erikabele@apache.org wrote:
> erikabele 2002/08/24 15:25:16
>
> Modified: docs/error HTTP_BAD_GATEWAY.html.var
> HTTP_INTERNAL_SERVER_ERROR.html.var
> docs/error/include top.html
> Log:
> Added encoding="none" for the ssi-output of REDIRECT_ERROR_NOTES.
> This fixes the output of HTML-tags through the above env-var (e.g.
> <p> instead of <p>).
Hmmm... We need a security-review of this change. Is it possible in any
way for the client to insert something into REDIRECT_ERROR_NOTES? If
so, this change must be reversed, because it opens a
Cross-site-scripting vulnerability.
I don't know the answer, but we need to be careful here.
Where are the <p> tags coming from, anyway? I thought ERROR_NOTES was
plain text.
Joshua.
---------------------------------------------------------------------
To unsubscribe, e-mail: docs-unsubscribe@httpd.apache.org
For additional commands, e-mail: docs-help@httpd.apache.org
Possible css vulnerability part II
Posted by Erik Abele <er...@codefaktor.de>.
After looking a second time in the code I realized that all calls to
ap_log_rerror with the APLOG_TOCLIENT state are properly escaped in
server/log.c. Therefore it should be safe to use
apr_filename_of_pathname(r->filename) unescaped in the mentioned places:
> #### Possible places for css vulnerabilities
> ####
> #### I'm quite not sure, if r->method and apr_filename_of_pathname(r->filename)
> #### are safe to use unescaped in these places, but I'm sure that it is not
> #### safe to use apr_table_get(r->headers_in, "Expect") unescaped!
>
> #### modules/generators/mod_cgi.c line 470 & mod_cgid.c line 650:
> #### apr_filename_of_pathname(r->filename) not escaped
>
> rc = ap_os_create_privileged_process(r, procnew, argv0, argv,
> (const char * const *)env,
> procattr, ptrans);
>
> if (rc != APR_SUCCESS) {
> /* Bad things happened. Everyone should have cleaned up. */
> ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_TOCLIENT, rc, r,
> "couldn't create child process: %d: %s", rc,
> apr_filename_of_pathname(r->filename));
> }
>
> #### server/util_script.c line 457:
> #### apr_filename_of_pathname(r->filename) not escaped
>
> if ((*getsfunc) (w, MAX_STRING_LEN - 1, getsfunc_data) == 0) {
> ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_TOCLIENT, 0, r,
> "Premature end of script headers: %s",
> apr_filename_of_pathname(r->filename));
> return HTTP_INTERNAL_SERVER_ERROR;
> }
>
> #### server/util_script.c line 551:
> #### apr_filename_of_pathname(r->filename) not escaped
>
> ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_TOCLIENT, 0, r,
> "%s: %s", malformed,
> apr_filename_of_pathname(r->filename));
> return HTTP_INTERNAL_SERVER_ERROR;
>
Erik
---------------------------------------------------------------------
To unsubscribe, e-mail: docs-unsubscribe@httpd.apache.org
For additional commands, e-mail: docs-help@httpd.apache.org
Possible css vulnerability
Posted by Erik Abele <er...@codefaktor.de>.
Joshua Slive wrote:
> erikabele@apache.org wrote:
> > erikabele 2002/08/24 15:25:16
> >
> > Modified: docs/error HTTP_BAD_GATEWAY.html.var
> > HTTP_INTERNAL_SERVER_ERROR.html.var
> > docs/error/include top.html
> > Log:
> > Added encoding="none" for the ssi-output of REDIRECT_ERROR_NOTES.
> > This fixes the output of HTML-tags through the above env-var (e.g.
> > <p> instead of <p>).
>
> Hmmm... We need a security-review of this change. Is it possible
in any
> way for the client to insert something into REDIRECT_ERROR_NOTES? If
> so, this change must be reversed, because it opens a
> Cross-site-scripting vulnerability.
>
> I don't know the answer, but we need to be careful here.
Hi Joshua !
A grep for 'error-notes' and 'APLOG_TOCLIENT' on the whole source tree
revealed all the places, in which the (REDIRECT_)ERROR_NOTES env-var is
set for output. I inspected all these codesections thoroughly and found
out, that almost every single output, which was coming from the client-side
(for example bad URLs) and is _not_ hardcoded, is properly escaped with
ap_escape_html().
There are some places where the escaping with ap_escape_html() is
missing. Attached a file with all the places I've found. IMO we should
consider escaping these too, since they are used for example in the
canned error messages and therefore should be properly escaped anyway!
I'm definetely not a specialist in css issues and therefore would
greatly appreciate some thoughts from the big boys. IMO
it should be safe for us to keep encoding="none" in the ssi-echos, if
all, possibly malicious, input from the client-side is properly escaped
in the codeplaces where the ERROR_NOTES env-var gets set.
What do you think? Am I missing something?
BTW, while browsing through the source I discovered a place in
proxy_util.c (line 612), where the output isn't XHTMLized so far:
apr_table_setn(r->notes, "error-notes",
apr_pstrcat(r->pool,
"The proxy server could not handle the request "
"<EM><A HREF=\"", ap_escape_uri(r->pool, r->uri),
"\">", ap_escape_html(r->pool, r->method),
" ",
ap_escape_html(r->pool, r->uri), "</A></EM>.<P>\n"
"Reason: <STRONG>",
ap_escape_html(r->pool, message),
"</STRONG>", NULL));
Since all other output and the error documents are valid XHTML, I think
we should change this too. I will prepare a patch for this and post it
on the dev list.
BTW, I inspected the following files:
./modules/generators/mod_cgi.c
./modules/generators/mod_cgid.c
./server/util_script.c
./modules/http/http_protocol.c
./modules/http/http_request.c
./modules/proxy/proxy_util.c
./modules/ssl/ssl_engine_kernel.c
./server/config.c
./server/log.c
./server/protocol.c
and some more, but they are not relevant at all.
Erik :-)