94551 - Need to escape content in about:cache-entry

Status: VERIFIED FIXED

(Core :: Security, defect, P2)

Description

[Mitchell Stoltz (not reading bugmail)]

There is potential problem with about:cache-entry - though not serious it may allow
tracking whether the user has visited a particular URL.
The problem is that about:cache-entry does not escape "response-head:"
and the web server may embed javascript in it which may check for cache-entry: urls.
The good news is about:cache-entry seems to not be executed in chrome.
It is possible to open about:cache-entry?* by playing with "about:blank" documents.
I have tested this with netcat (nc) which emulates malicous web server.
Think that fixing this is easy - just escape everything in about:cache-entry

Comment 1

[Mitchell Stoltz (not reading bugmail)]

(The above is from Georgi.)

We should look at other about: content for similar problems. By itself there's
not much you can do with this, but combined with 91714 this becomes a moderately
serious vulnerability.

Comment 2

[Mitchell Stoltz (not reading bugmail)]

With 91714 fixed this is not currently exploitable, so moving it to 0.9.5. We
should have a nonspoofable way of escaping content to keep it from being
interpreted as HTML; this could be used in a number of places.

Comment 3

[Jesse Ruderman]

See bug 90386 for another place where a non-spoofable htmlEscape might be 
useful.  (Wouldn't it make more sense to use DOM2 createTextNode instead, 
though, so the results aren't parsed at all?)

Comment 4

[Mitchell Stoltz (not reading bugmail)]

time marches on. Retargeting to 0.9.6.

Comment 5

[Mitchell Stoltz (not reading bugmail)]

Moving the most time-critical bugs and minor security fixes to 0.9.7

Comment 6

[Alexey Chernyak]

A fix for this could be using text/plain instead of text/html to display
about:cache-entry. However that would remove such formatting as link, bold
fields and <hr>.

Another alternative is creating an text/plain <object> inside of this page and
putting the response header in there. Is that possible?

And yet another (and probably most preferable) way would be inserting a Text
node into DOM after the page is created. However this would have to be done not
through JavaScript, but from C++. Is that possible?

Comment 7

[Mitchell Stoltz (not reading bugmail)]

Setting security-sensitive flag again; it was cleared because of bug 107718.

Comment 8

[Mitchell Stoltz (not reading bugmail)]

Attachment: Patch - Escape HTML tags when writing cache metadata to about:cache-entry pages

Comment 9

[Mitchell Stoltz (not reading bugmail)]

Still need to test this fix, and for that I need to configure a web server to
serve some bogus headers.

Also, what alternate representations of <, >, and & do I need to worry about?
What about characters with the high bit set? Or Unicode characters?

CCing darin, who appears to have written the about:cache code.

Comment 10

[Bradley Baetz (:bbaetz)]

Your patch leaks the return val, doesn't it?

Those three are all you have to worry about, I think. It may be possible for
unicode chars to cause teh charset to be incorrectly detected, I guess. I don't
think there are any security ramifications, though.

However, you should probably use nsEscapeHTML, defined in nsEscape.h. That also
escapes " - not sure why.

Comment 11

[Mitchell Stoltz (not reading bugmail)]

Whoops - it sure does. I'll try nsEscape.

Comment 12

[Mitchell Stoltz (not reading bugmail)]

Attachment: Patch take 2 - use existing escape function

Comment 13

[Bradley Baetz (:bbaetz)]

Oh, and adding the cclist to the can-see-this-bug group, since you wanted
darin's comment, but he can't see this as is...

Comment 14

[Bradley Baetz (:bbaetz)]

Comment on attachment 58612 [details] [diff] [review]
Patch take 2 - use existing escape function

r=bbaetz

Comment 15

[Gagan]

Comment on attachment 58612 [details] [diff] [review]
Patch take 2 - use existing escape function

r=gagan as well...

Comment 16

[Gagan]

Comment on attachment 58612 [details] [diff] [review]
Patch take 2 - use existing escape function

sr=darin

Comment 17

[Darin Fisher]

this looks fine to me... just too bad that escaping html requires a strdup even
when escaping isn't really required :(

Comment 18

[Bradley Baetz (:bbaetz)]

darin: I think theres a bug somewhere on  moving these functions to Real String
APIs. I don't know if that could help, though, although I guess with COW you
could do some short circuiting.

Comment 19

[Mitchell Stoltz (not reading bugmail)]

checked in.

Comment 20

[Mitchell Stoltz (not reading bugmail)]

To verify this, visit http://grey:4321/bad, and then visit about:cache, click
Disk Cache, and click the link for http://grey:4321/bad. The resulting page
should not generate an alert.

Comment 21

[bsharma]

Verified on
build: 2001-12-11-o3-Trunk
platform: Win NT

Performed the above steps and did not see the alert dialog.