Ticket #2336 (closed defect: fixed)
possible security hole in default error handler
| Reported by: | diefans | Owned by: | pedersen |
|---|---|---|---|
| Priority: | highest | Milestone: | 2.0.* bugfix |
| Component: | TurboGears | Version: | 2.0 |
| Severity: | critical | Keywords: | javascript injection |
| Cc: |
Description (last modified by jorge.vargas) (diff)
Hello,
It is possible to inject executable Javascript-Code into an error page generated by the default ErrorController?.document method. Just put the code into the message URL parameter.
This is because:
- the error handler uses manipulatable request parameters
- the template shows those possible manipulated parameters as XML and not as escaped HTML/XML (IMHO quite irresponsible)
Since the error handling will be the last point where developers put attention to, even some will ignore it completely or use these defaults, this could be a sleeping vulnerablity to many of them.
Attachments
-
screenshot1.png
(32.5 KB) -
added by diefans 3 years ago.
- just to see
Change History
comment:1 Changed 3 years ago by jorge.vargas
- Version changed from 2.0b7 to 2.0
- Description modified (diff)
- Milestone set to 2.0
comment:2 Changed 3 years ago by jorge.vargas
Could you provide an example call to that controller that will display an alert() window? From looking at how it is handle it should not be possible to put that in the URL as it is an indirection call to the errormiddleware. Neither GET or POST.
http://localhost:8080/error/document?status_int=3
returns AttributeError?: 'NoneType?' object has no attribute 'status_int' with WebError? on.
comment:3 Changed 3 years ago by diefans
lets say you have an application where you serve files and when a wrong file id is requested you serve an 404 error by calling:
raise HTTPNotFound
on that page you can append a message parameter like
http://localhost:8080/file?id=invalid&message=%3Cscript%20type=%22text/javascript%22%3Ealert(%27test%27)%3C/script%3E
this is my error document method as it was created by quickstart. As you can see the URL parameter "message" is forwarded to the template as it is:
def document(self, *args, **kwargs):
"""Render the error document"""
resp = request.environ.get('pylons.original_response')
default_message = ("<p>We're sorry but we weren't able to process "
" this request.</p>")
values = dict(prefix=request.environ.get('SCRIPT_NAME', ''),
code=request.params.get('code', resp.status_int),
message=str(request.params.get('message', default_message)))
return values
this is the error template as it was created by quickstart:
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml"
xmlns:py="http://genshi.edgewall.org/"
xmlns:xi="http://www.w3.org/2001/XInclude">
<xi:include href="master.html" />
<head>
<meta content="text/html; charset=UTF-8" http-equiv="content-type" py:replace="''"/>
<title>A ${code} Error has Occurred </title>
</head>
<body>
<h1>Error ${code}</h1>
<div>${XML(message)}</div>
</body>
</html>
the relevant part is:
<div>${XML(message)}</div>
which means, that everything in message is considered to be valid XML and is inserted that way. At the end you have the following code in your error page:
<div id="content">
<h1>Error 404</h1>
<div><script type="text/javascript">alert('test')</script></div>
<div class="clearingdiv"></div>
<!-- End of content -->
</div>
it's a security hole, because unplanned and unwanted events may occur on that Page...
comment:4 Changed 2 years ago by Chris Arndt
- Owner set to mramm
This seems like a serious issue. Though to exploit it as an XSS attack the attacker must be able to plant a forged URL on the victim.
comment:5 Changed 2 years ago by diefans
Just imagine a Blog where a commentator is able to post links or to enter his "own" website...
comment:7 Changed 12 months ago by pedersen
- Owner changed from mramm to pedersen
- Status changed from new to assigned
Possible fix is located here. If everybody agrees, I will push this to SF, and copy fix to 2.1.
https://github.com/pedersen/tg2devtools/commit/fd8e0b640f367801f4e054a168b9cd90ce6724d9
comment:8 Changed 12 months ago by pedersen
Better fix located here. Unless someone can come up with a reason not to make this fix permanent (and/or a better patch), I'll commit this to SF by Monday night.
https://github.com/pedersen/tg2devtools/commit/b2f190fbfc5461fae88c3bbf03508c815f430ad4
comment:9 Changed 12 months ago by pedersen
Finalized fix. Unless an issue arises, I will complete the merge/push tomorrow night. Speak up now, people, if you don't like it.
Commits for 2.0 and 2.1 are here: https://github.com/pedersen/tg2devtools/commit/2fc970a669718a6bc9924e158775e5e60e78aa25 https://github.com/pedersen/tg2devtools/commit/528caef07f102c831311b4885a927f0216553f4d
comment:10 Changed 12 months ago by pedersen
- Status changed from assigned to closed
- Resolution set to fixed
The fix for this issue has been commited into git. It will be released in 2.0.4 and 2.1.1, and then stay around afterwards. Closing this ticket as the work is done.
I'm not entirely sure this is accurate. Will ask a genshi expert.