95735 - Node.attributes.getNamedItem bypasses capability policy check (NamedNodeMap)

Status: RESOLVED FIXED

(Core :: Security: CAPS, defect)

Description

[Jesse Ruderman]

Node.attributes.getNamedItem("attrname") returns an attribute node.  I can read 
and modify the attribute node's value even if I couldn't access Node.attrname 
directly.

Impact: blocks bug 84545, "Review message pane DOM for exploits", by allowing 
scripts to get around noAccess on baseURI, innerHTML, etc.

Comment 1

[Jesse Ruderman]

Attachment: testcase

Comment 2

[Mitchell Stoltz (not reading bugmail)]

Moving from NS-Confidential to the security group.

Comment 3

[Mitchell Stoltz (not reading bugmail)]

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

Comment 4

[Bradley Baetz (:bbaetz)]

Mass change: It appears that several bugs got accidentally opened up as part of
a mass change - see bug 107718 (now fixed). Moving back to security-sensitive
group. These were open for about 10 days.

Comment 5

[Mitchell Stoltz (not reading bugmail)]

Mozilla1.0

Comment 6

[Mitchell Stoltz (not reading bugmail)]

Mozilla1.1

Comment 7

[Brendan Eich [:brendan]]

Dan, gifting to you, not sure how bad this is (can an evil hidden frame script
get a credit card # or SSN if it knows the form layout?), but if we need to hold
1.0 for it, let drivers know ASAP.

/be

Comment 8

[Mitchell Stoltz (not reading bugmail)]

> (can an evil hidden frame script
> get a credit card # or SSN if it knows the form layout?)

No. The same-origin policy will still apply. This bug could allow bypassing some
of the DOM restrictions in MailNews which were put in because of the "wiretap"
bug, but this cannot be used to steal data from 3rd-party web pages, so I don't
consider it a 1.0 stopper.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think this should be a 1.2beta blocker. Holes like this have a tendency to
suddenly become important in unexpected ways. We should fix it now before we
have to do a firedrill.

Comment 10

[Christopher Blizzard (:blizzard)]

No work on this bug yet?

Comment 11

[Mitchell Stoltz (not reading bugmail)]

I'll take this back for 1.3.

Comment 12

[Christopher Blizzard (:blizzard)]

Why is this pushed out to 1.3?  This is on the 1.2 list.  We're likely to hold
for this since it's a security bug.

Comment 13

[Mitchell Stoltz (not reading bugmail)]

This bug is minor; I don't recommend holding 1.2 for it. I put it to 1.3 because
that's when I'll have time to work on it.

Comment 14

[Mitchell Stoltz (not reading bugmail)]

Clearing milestone for now.

Comment 15

[Mitchell Stoltz (not reading bugmail)]

OK, turns out we fixed this for mailnews by blocking the "attribute" property of
all objects, so scripts can't use NamedNodeMap functions to read attributes of
mailed documents. We also need to protect GetAttributeNS and GetNamedItemNS.

The more general problem only comes into play when configurable security
policies are used. The problem is that there's more than one path to getting at
the attributes of an element. In Jesse's testcase, blocking access to
HTMLAnchorElement.href is insufficient, because the HREF attribute can still be
accessed using attributes.getNamedItem('href').

We could fix this in code by adding additional security checks to tie the
multiple pathcs to an attribute together under a single security pref. However,
jst suggests that we don't do this. The functions of the attributes object are
used extensively by chrome; adding security checks to them would have a
detrimental effect on performance. Degrading performance for everyone to protect
a few people using configurable security policies in a particular way is not a
good trade-off when a simple alternative is available: in order to protect
HTMLImageElement.src, for example you need to protect .src, .getAttribute,
.getAttributeNS, and .attributes by setting "noAccess" prefs for each one of
those. I will add a note about that to the Configurable Policies instruction page.

Comment 16

[Mitchell Stoltz (not reading bugmail)]

Attachment: Patch - protect a couple more properties of mail documents

Comment 17

[Mitchell Stoltz (not reading bugmail)]

Comment on attachment 115815 [details] [diff] [review]
Patch - protect a couple more properties of mail documents

r=mstoltz (patch is by jst)

Comment 18

[Mitchell Stoltz (not reading bugmail)]

Fix checked in.

Comment 19

[Mitchell Stoltz (not reading bugmail)]

.