Last Comment Bug 135267 - Reading files cross-host using styles
: Reading files cross-host using styles
Status: VERIFIED FIXED
[sg]
:
Product: Core
Classification: Components
Component: Security (show other bugs)
: Trunk
: x86 Windows 2000
: -- normal (vote)
: mozilla1.0.1
Assigned To: Mitchell Stoltz (not reading bugmail)
: bsharma
Mentors:
Depends on:
Blocks: 143047
  Show dependency treegraph
 
Reported: 2002-04-03 14:23 PST by Mitchell Stoltz (not reading bugmail)
Modified: 2002-11-07 08:14 PST (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Example (445 bytes, text/html)
2002-04-03 14:24 PST, Mitchell Stoltz (not reading bugmail)
no flags Details
Patch - block cross-origin DOM access to style rule collections (2.02 KB, patch)
2002-05-28 20:20 PDT, Mitchell Stoltz (not reading bugmail)
security-bugs: review+
scc: superreview+
jud: approval+
Details | Diff | Review

Description Mitchell Stoltz (not reading bugmail) 2002-04-03 14:23:27 PST
Got this for IE on bugtraq this morning:
Reading portions of local files, depending on structure
http://security.greymagic.com/adv/gm004-ie/

Mozilla is partially affected by a modification of this.
Thru stylesheets it is possible to read parts of remote files (does not
work with local files) in case the file contains curley bracket - "{".

The attached file "styleread.html" reads the stylesheets from netscape.com
Comment 1 Mitchell Stoltz (not reading bugmail) 2002-04-03 14:24:08 PST
Created attachment 77527 [details]
Example
Comment 2 georgi - hopefully not receiving bugspam 2002-05-08 01:47:57 PDT
Can't this be solved with the javascript security policy by setting
document.styleSheets to sameorigin?
Comment 3 Brendan Eich [:brendan] 2002-05-22 13:43:09 PDT
Cc'ing helpers.

/be
Comment 4 Mitchell Stoltz (not reading bugmail) 2002-05-28 20:20:13 PDT
Created attachment 85380 [details] [diff] [review]
Patch - block cross-origin DOM access to style rule collections

I've verified that this fixes the problem, but I'm not sure what other tests to
run (to look for regressions).
Comment 5 Scott Collins 2002-05-28 20:39:41 PDT
Comment on attachment 85380 [details] [diff] [review]
Patch - block cross-origin DOM access to style rule collections

sr=scc
Comment 6 Mitchell Stoltz (not reading bugmail) 2002-05-28 20:51:29 PDT
Comment on attachment 85380 [details] [diff] [review]
Patch - block cross-origin DOM access to style rule collections

r=dveditz. We still need testing, though, and I'm not sure what to test.
Comment 7 Mike Shaver (:shaver -- probably not reading bugmail closely) 2002-05-28 20:54:10 PDT
My testing thoughts, from email:

* Visit a page with multiple stylesheets (alternate, etc.) and make sure that
  they are switchable with View->Use Style.
  http://www.alistapart.com/stories/alternate/ is such a page.

* DOM inspector, visit a page with multiple stylesheets and make sure that
  they all appear in the "Stylesheets" version of the left view.

* content/xml/tests/toc and content/xml/tests/books both seem to use
  the .styleSheets property, give them a spin.

* test16 from viewer, too.

* chatzilla uses it too, if you want to be really thorough.
Comment 8 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2002-05-28 21:04:46 PDT
Actually, I don't think this code should be triggered by any of those things. 
It should only be triggered by CSSOM usage, which is somewhat rare (since the
CSSOM is horribly designed, etc., etc.).

bzbarsky would know exactly what would use this, and probably be able to point
to testcases.  He'd also know if there were other access points for the same or
similar information (although I suspect you've already checked, to some extent).

I don't *think* (off the top of my head, without looking at the code) that this
bug is as serious as the vulnerablility in IE, since our CSS stylesheet data
structures only store what's valid CSS (and we then reconstruct .cssText from
our data structures), at least for the most part.  That may not be true for
everything, though (CSSUnknownRule?).
Comment 9 Mitchell Stoltz (not reading bugmail) 2002-05-28 21:44:20 PDT
That's what I figured too. This can only be used to steal data that looks
something like valid CSS. Georgi's testcase, above, only reads style data from
netscape.com, not scripts or any other sort of content.

I tried Shaver's recommended tests, just for the halibut, and everything appears
to work fine. So, I need to know if Mozilla wants this patch for 1.0 or not. Do
you think an attacker could glean valuable data from inline CSS on a bank's
page, for example?

I'll check this in if you want it, just let me know within a few hours.
Comment 10 Boris Zbarsky [:bz] 2002-05-28 22:33:10 PDT
We don't implement CSSUnknownRule and I hope we never do.  There are no 
scriptable access points for the rules list other than this that I know of, and 
there are no scriptable access points for rules other than the rules list.  So 
the patch looks good to me....

Things to test, possibly:

Chrome:

1)  The stylesheet list in Inspector (click on a sheet; I _think_ it should
    somehow make the rules visible....)
2)  Switching between beginner/intermediate/advanced user levels in XMLTerm
    (easy way to exercise this code).

Content:
1)  
http://lxr.mozilla.org/seamonkey/source/extensions/xmlterm/ui/content/styletest.
html (uses dump() to dump out the sheet).
2) http://bugzilla.mozilla.org/attachment.cgi?id=56048&action=view
3) http://bugzilla.mozilla.org/attachment.cgi?id=84012&action=view
4) http://www.xs4all.nl/~ppk/js/changess.html (the "Try changing the colour of
   all PRE's to blue" link)
Comment 11 Brendan Eich [:brendan] 2002-05-29 09:21:14 PDT
Comment on attachment 85380 [details] [diff] [review]
Patch - block cross-origin DOM access to style rule collections

>+  JSContext *cx = nsnull;
>+  nsresult rv = NS_OK;
>+
>+  rv = stack->Peek(&cx);

Nit: no need to initialize rv if it's immediately set to another value.

/be
Comment 12 Mike Shaver (:shaver -- probably not reading bugmail closely) 2002-05-29 09:53:47 PDT
Comment on attachment 85380 [details] [diff] [review]
Patch - block cross-origin DOM access to style rule collections

>+  if (NS_FAILED(rv)) {
>+    return rv;
>+  }

While we're nitting, this is inconsistent with the other
the uses of NS_ENSURE_TRUE, and the brace style of the 
rest of the patch for single-line then clauses.
Comment 13 Daniel Glazman (:glazou) 2002-05-29 15:00:06 PDT
Given the fact that we don't have CSSUnknownRule, that cssText can only disclose
CSS valid rules, that it is unlikely that a class name in a stylesheet will
contain a super-important information like a credit card number or a password, that
CSS files do not store OS-related information, I do believe
that we have no super-urgent mega-important issue here (to say the least).
There is no possible comparison between the way we could be hit by this problem and
the way IE is currently hit by it.
Comment 14 scottputterman 2002-06-06 23:22:18 PDT
Assuming the patch is done, would it be possible to land this in 1.1alpha and
get some of the test cases mentioned in the bug verified before landing on the
branch?
Comment 15 Mitchell Stoltz (not reading bugmail) 2002-06-14 19:34:04 PDT
Fix checked in on the trunk - please verify this ASAP.
Comment 16 Daniel Glazman (:glazou) 2002-06-17 01:55:28 PDT
IMPORTANT : Am I correct if I say that if I create a document using a W3C
stylesheet, publish this document on my personal web site, the CSS OM won't be
able to list the rules in the attached stylesheet with this patch in ?

If I am correct, this is a too strong restriction and I don't think this should
land. Correction : I am sure this should not land.
Comment 17 Heikki Toivonen (remove -bugzilla when emailing directly) 2002-06-17 10:29:17 PDT
Daniel, I am not sure what you mean (CSS OM?). 

I think this does the same thing as the fix for XSLT (although I have not
checked this patch yet). The XSLT patch allows (I think, I should check again) a
document on site A to read in a stylesheet from site B, but the stylesheet at
site B can only read additional files from site A. Do you want to: 1) have site
B stylesheet only be able to read in files at site B, 2) have site B stylesheet
be able to read in files from sites A and B?
Comment 18 Mitchell Stoltz (not reading bugmail) 2002-06-17 12:02:53 PDT
From Bindu:

 Verified the http://bugzilla.mozilla.org/show_bug.cgi?id=135267 on today's
trunk build.

    Steps:
    1. Clciked on the attachment example.
    2. Wait for page to load.
    3. Saw an exception in the console and no alert.

Exception:
Error: uncaught exception: [Exception... "Access to restricted URI denied" 
code: "1012" nsresult: "0x805303f4 (NS_ERROR_DOM_BAD_URI)"  location:
"http://bugzilla.mozilla.org/attachment.cgi?id=77527&action=view Line: 15"]

Marking Verified.
Comment 19 Mitchell Stoltz (not reading bugmail) 2002-06-17 12:03:59 PDT
Let's try that again...
Comment 20 Boris Zbarsky [:bz] 2002-06-17 12:07:37 PDT
Daniel, your statement in comment #16 is exactly correct.  Consider the
following:  If you have an <iframe> on your site in which you include the W3C
site, you will not be able to access the DOM of the W3C site.  This patch merely
applies a similar restriction to stylesheets.
Comment 21 Daniel Veditz [:dveditz] 2002-06-17 15:35:22 PDT
But those aren't exactly equivalent. In the foreign <iframe> the user could
start inputting data or otherwise interacting with the site, and we want to
prevent eavesdropping (the initial DOM state is uninteresting, the attacker
could just visit the site himself).

Granted I'm not sure why you'd want to walk the style rules in some foreign
sheet you've applied to your document, but I don't see how it's leaking any
information. Maybe a CheckLoadURI to block file:// sheets would be sufficient.
Comment 22 Boris Zbarsky [:bz] 2002-06-17 15:43:42 PDT
> the attacker could just visit the site himself

Not if the site requires authentication (cookies, HTTP auth, etc)...
Comment 23 Daniel Veditz [:dveditz] 2002-06-17 16:59:41 PDT
Exactly, that's what I was trying to say. The page as viewable by the attacker
is obviously uninteresting or he would just go view it; something about adding
the user into the mix makes it worth trying to eavesdrop.

I don't see how this carries into the CSS case. If the "attacker" can style his
page using the foreign sheet then he obviously has access to the sheet. If we're
worried that he only has access because he's borrowing the user's cookie auth
then maybe we should block his ability to style using that sheet entirely, but I
don't see the point of applying the style yet blocking script ability to see
them. If you apply the sheet you can do things like getComputedStyle() to try to
figure out what was in it.

(We already block the ability to style using local file:// sheets from remote
content, sorry for not checking before suggesting it.)
Comment 24 Boris Zbarsky [:bz] 2002-06-17 17:05:47 PDT
> If the "attacker" can style his page using the foreign sheet then he obviously
> has access to the sheet.

No... All that means is that at pageload we put up a password dialog
_for_the_sheet_ and the user logged in to the site the sheet is being gotten
from via this dialog.  Or that the user has a stored cookie that allows access
to this sheet that we send with the request.

This is really no different from the <iframe> case, as far as I can tell...

Oh, and getComputedStyle is a very imprecise tool for getting the original
stylesheet data. It's a darned crappy one in fact.
Comment 25 Daniel Glazman (:glazou) 2002-06-18 01:21:56 PDT
Dan and all : in a former professionnal life at EDF, I had to deal with a dozen
of corporate web sites where the pages had to be presented with a corporate
stylesheet available from a thirtheenth web site. No local copy of the sheet was
allowed to be sure that an update of the sheet would affect all pages.
In such a environment, no web page will be able to browse the contents of the
style sheet using the CSS OM because of the fix for the current bug.

I am saying that this restriction is (a) too strong (b) not in the spirit of CSS
(b) almost useless because all that can be retrieved by this mean is CSS style
rules that have been validated by the CSS parser and re-generated. There is no
way in Mozilla the document can access the SOURCE of the file.
Conclusion : only CSS rules in the file will be made available through CSS OM.
In particular, if the file is *not* a CSS rule, the CSS OM will show nothing at
all.

This is clearly different from an IFRAME and DOM because a sheet does not contain
**content**, only stylistic rules. One does not store a password in a CSS rule,
nor a credit card number.

Let me rephrase with perhaps stronger words, but more perhaps also more
accurate : Ok, I can build a document here able to detect that www.netscape.com
has in itself a CSS rule defining yellow color for a div ! So what ?

Isn't the solution worse than the problem ?

Comment 26 Mitchell Stoltz (not reading bugmail) 2002-06-18 11:41:38 PDT
Daniel,
   You're beginning to convince me. However, I'm worried about future changes to
CSS or the CSSOM opening up security holes. Currently, as you say, the CSSOM
only exposes parsed style rules, not source, and no other information about the
document. Will this always be so? Will a future version of CSS possibly allow
stylesheets to contain more information that might be of interest to an
attacker? If so, I can easily see someone adding additional functionality to CSS
without being aware of the security consequences. This patch makes us future-proof.

Also, are we absolutely sure that the CSSOM won't pick up other content in the
page that looks like CSS but isn't? Like scripts, maybe? Is that possible?
Comment 27 Daniel Glazman (:glazou) 2002-06-18 14:09:46 PDT
> You're beginning to convince me. However, I'm worried about future changes to
> CSS or the CSSOM opening up security holes. Currently, as you say, the CSSOM
> only exposes parsed style rules, not source, and no other information about the
> document. Will this always be so?

Mitch, I am now the editor of the future versions of the CSS OM, in the CSS
WG. We have in the CSS WG two "keepers of the temple", the two inventors of
CSS. Vidur can confirm that these two guys will never let CSS include anything
else but declarative stylistic stuff. When we tried together a few years ago to
add scripts to CSS sheets, we faced a so strong opposition that it went
fast into /dev/null.

> Will a future version of CSS possibly allow stylesheets to contain more
> information that might be of interest to an
> attacker?

No. The CSS WG also cares for security issues and any feature that could expose
a hole should be blocked during review process. Again, I am the editor of the
document, we should be able to do something here ;-)
If we all agree here, I could raise the issue of this bug to the CSS WG and
I am pretty sure that the WG will come up with a clarification about
CSSUnknownRule.

> If so, I can easily see someone adding additional functionality to CSS
> without being aware of the security consequences. This patch makes us 
> future-proof.

Frm my point of view, the fact that we don't implement CSSUnkownRule is far
enough. We are proof to all non-CSS content that a malicious document would
try to reach through stylesheet linking.

> Also, are we absolutely sure that the CSSOM won't pick up other content in the
> page that looks like CSS but isn't? Like scripts, maybe? Is that possible?

Well, since our implementation of the CSS OM will just not keep a rule or
selector or declaration if it's not a valid CSS rule, I guess that I can answer
without any fear "yes, we are sure it can't pick up other content".

I recommend (a) backing out this patch (b) find a less intrusive fix or explain,
perhaps through an offical PR, that the security warning does not apply to
Mozilla/Netscape because we don't implement CSSUnkownRule (explanations above).

(I will be away from the office during the following weeks and will not be able
 to contribute to this discussion...)
Comment 28 Boris Zbarsky [:bz] 2002-06-18 15:12:57 PDT
> any feature that could expose a hole should be blocked during review process

Does that include not exposing CSS comments via the CSSOM, ever?
Comment 29 Judson Valeski 2002-06-21 08:45:48 PDT
please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.
Comment 30 Jaime Rodriguez, Jr. 2002-06-22 01:54:08 PDT
adt1.0.1 (on ADT's behalf) approval for checkin into the 1.0 branch. pls check
this in asap, then add the "fixed1.0.1" keyword.
Comment 31 scottputterman 2002-07-01 17:47:45 PDT
Please check this in today.  If it's already been checked in, please replace the
mozilla1.0.1+ keyword with fixed1.0.1+.
Comment 32 Boris Zbarsky [:bz] 2002-07-01 23:53:39 PDT
checked in on the branch.
Comment 33 Daniel Glazman (:glazou) 2002-07-22 01:22:35 PDT
So the answer to my comment #27 was "check that in everywhere" ?!?!?
Sorry to say so, but we took the worst direction here IMHO.
Comment 34 bsharma 2002-07-23 10:47:50 PDT
Verified on 2002-07-23-branch build on Win 2000.

Steps:
    1. Clciked on the attachment example.
    2. Wait for page to load.
    3. Saw an exception in the console and no alert.

Exception:
Error: uncaught exception: [Exception... "Access to restricted URI denied" 
code: "1012" nsresult: "0x805303f4 (NS_ERROR_DOM_BAD_URI)"  location:
"http://bugzilla.mozilla.org/attachment.cgi?id=77527&action=view Line: 15"]

Also loaded the styles pointed out in comment #7
* Visit a page with multiple stylesheets (alternate, etc.) and make sure that
  they are switchable with View->Use Style.
  http://www.alistapart.com/stories/alternate/ is such a page.

* content/xml/tests/toc and content/xml/tests/books both seem to use
  the .styleSheets property, give them a spin.
Comment 35 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2002-11-07 08:14:43 PST
Just adding an annotation so I can find this bug again.

FTR I agree with the resolution. The content of stylesheets could be sensitive
in some bizarre cases. E.g., monitor the classes used in the style sheet for
Company X's intranet home page, and notice when someone adds the class
"earnings-alert" or changes its style. Ideally we would turn off cross-domain
style sheets completely, but since that's too restrictive, this is the best we
can do, and it's a lot better than nothing. It also protects us against future
CSSOM changes --- or bugs in our CSS implementation.

Note You need to log in before you can comment on or make changes to this bug.