Closed Bug 800548 Opened 12 years ago Closed 8 years ago

IE-only XSS in style attribute on Mozilla Developer Network

Categories

(developer.mozilla.org :: Security, defect, P2)

Product:
developer.mozilla.org
Component:
Security
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: yaroslav.c7s, Assigned: groovecoder)

References

Details

(Keywords: wsec-xss, Whiteboard: [site:developer.mozilla.org][type:bug]) 

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.4 (KHTML, like Gecko) Chrome/22.0.1229.94 Safari/537.4

Steps to reproduce:

XSS in Mozilla Developer Network.
Work in IE 6 - 10(in ie after 7 - in Compatibility Mode).
No filtering css expression in style attribute.

Steps to reproduce the vulnerability:
1. log in developer.mozilla.org
2. open profile page
3. click "Docs user page" (near avatar)
4. click html mode (source)
5. paste <div style="width:expression\000028alert\000028\000027xss\000027\000029\000029;">fdf</div>
6. save
7. open page in IE.




Actual results:

No filtering css expression in style attribute.
OR
<div style="width:expression(alert('xss'))">fdf</div>
Status: UNCONFIRMED → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
This is not dublicate!
Look again, please.

Vulnerable page: https://developer.mozilla.org/ru/docs/new?slug=User%3A
Yep, sorry, that was a mis.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: DUPLICATE → ---
I've tried multiple times but I can't seem to get this this to repro.
This looks like an IE bug to me; IE is allowing JavaScript from within CSS, which is non-standard.
IE does half the web browsing out there, sites like MDN don't have the luxury of saying "don't come here with IE".

If it were just IE 6 we could blow it off, but not if it's modern IE too. Why is it going into compat mode? preventing that is probably easier than writing adequate filtering code if it doesn't exist yet.

Why do we even allow style?
Summary: XSS in Mozilla Developer Network → IE-only XSS in Mozilla Developer Network
We allow style because this is a web site whose purpose it is to document and demonstrate the use of CSS and HTML; we need to be able to, you know, use CSS and HTML in order to demonstrate it.

We do have plans to mitigate this; we're working on code for a new sample management setup where the actual execution of the samples will be done on a separate domain, in a sandboxed iframe. This should reduce or eliminate the risks. This should be ready to deploy sometime in the next 2-3 weeks (most of it has already landed; we just need the IT side of things to be done).
Once that lands, we would beef up our filters to prevent most CSS and HTML from being used within the docs themselves, btw.
My reward?
Hi Yaroslav,

This will be reviewed by the bounty team soon.
Flags: sec-bounty?
Hi Yaroslav, I am testing this now and MDN appears to have filtered out all of the examples you provided. Can you let me know if the XSS is still working for you? It's possible MDN was updated. Thanks!
Hi!
This realy work, don't fix:
demo (all ie version): https://developer.mozilla.org/ru/docs/User:test_akk_new
(example in "Duplicate of this bug: 801046")
Flags: needinfo?
I'll try to reproduce this before our next bounty meeting (12/03)
Flags: needinfo?
Any status, David?
Flags: needinfo?(dchan+bugzilla)
Status: REOPENED → UNCONFIRMED
Ever confirmed: false
I just confirmed this works in IE9 in compatibility mode. 

The above steps provided by Yaroslav work with the following additions.
8. Bring up the IE developer tools (F12)
9. Change "Browser mode" to "IE9 Compatibility mode" OR change "Document Mode" to "Quirks mode"

You can test on this page
https://developer.allizom.org/en-US/docs/800500

I don't know if it is possible to force the later versions of IE to fallback to quirks mode with our current configuration. The source contains the following IE specific directive

  <!--[if IE]>
  <meta http-equiv="imagetoolbar" content="no">
  <meta http-equiv="X-UA-Compatible" content="IE=Edge">
  <script src="/media/js/html5.js"></script>
  <![endif]-->

and 

<!DOCTYPE html>

The X-UA-Compatible header /should/ force enable standards mode on supported versions of IE (IE8+)
MDN pages also specifies <!DOCTYPE html> which forces standards mode

http://msdn.microsoft.com/en-us/library/jj676915%28v=vs.85%29.aspx
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(dchan+bugzilla)
Assigning to Eric since he mentioned a possible fix in comment 8 but the bug still seems to be in the site. Please route to the appropriate developer.
Assignee: nobody → eshepherd
Flags: sec-bounty? → sec-bounty+
Component: User profiles → Security
Whiteboard: [site:developer.mozilla.org]
Summary: IE-only XSS in Mozilla Developer Network → IE-only XSS in style attribute on Mozilla Developer Network
I'm not able to reproduce this vulnerability in any IE mode on either https://developer.allizom.org/en-US/docs/800500 or on https://developer.mozilla.org/ru/docs/User:test_akk_new.

Visiting https://developer.mozilla.org/ru/docs/User:test_akk_new in IE9 mode or below *does* trigger XSS, but it's from bug 801046.
David, thoughts on comment 20? If the only problem stems from bug bug 801046, perhaps we should close this one.
Flags: needinfo?(dchan+bugzilla)
Whiteboard: [site:developer.mozilla.org] → [site:developer.mozilla.org][specification-like][type:bug]
Priority: -- → P1
(In reply to John Karahalis [:openjck] from comment #21)
> David, thoughts on comment 20? If the only problem stems from bug bug
> 801046, perhaps we should close this one.

I propose we just reject content if it contains IE specific directives (tags or attributes). That should fix both bugs.
Flags: needinfo?(dchan+bugzilla)
(In reply to David Chan [:dchan] from comment #22)
> I propose we just reject content if it contains IE specific directives (tags
> or attributes). That should fix both bugs.

MDN's permissive CSS rules make this a challenge. Is it OK to blacklist things like "expression" rather than taking a whitelist approach, like we normally would? I don't like it, but...
Ideally, we shouldn't be very permissive with CSS or markup - that's what bug 818966 is all about. But, addressing that is a long ways out.

That said, what does "IE specific directives (tags or attributes)" mean, specifically? Do we have a list somewhere on which to base a blacklist? Not sure how to proceed
Conditional comments are bad (I'm considering dropping the keep-comments option from Bleach entirely because I can't think of a robust-enough solution), but there are also IE-only CSS extensions that allow CSS to execute JS, like "width: expression(some JS goes here)"[1].

[1] http://perishablepress.com/maximum-and-minimum-height-and-width-in-internet-explorer/
Oh, I think I might see the issue: I don't think Bleach touches markup within HTML comments at all, whitelist or no. And, it just so happens IE supports a funky form of HTML comments that get transformed into real markup. This hits bug 801046, too.

What it seems like we should do is filter out HTML comments from rendered content altogether. I can think of no good reason to serve them up, and I can't think of any case where we actually want to allow IE-conditional-comments in MDN doco
(In reply to Les Orchard [:lorchard] from comment #28)
> Oh, I think I might see the issue: I don't think Bleach touches markup
> within HTML comments at all, whitelist or no. And, it just so happens IE
> supports a funky form of HTML comments that get transformed into real
> markup. This hits bug 801046, too.

Bleach's default is to set strip_comments=True for clean.

https://github.com/jsocol/bleach/blob/master/bleach/__init__.py#L94
(In reply to James Socol [:jsocol, :james] from comment #29)
>
> Bleach's default is to set strip_comments=True for clean.
> 
> https://github.com/jsocol/bleach/blob/master/bleach/__init__.py#L94

We have strip_comments=False - why, I can't remember. So... I guess let's change that to the default, and see what tests that breaks (if any)

https://github.com/mozilla/kuma/blob/master/apps/wiki/models.py#L377
https://github.com/mozilla/kuma/pull/960

That will strip all HTML comments from rendered content. Does that address all the issues here?
(In reply to Les Orchard [:lorchard] from comment #31)
> https://github.com/mozilla/kuma/pull/960
> 
> That will strip all HTML comments from rendered content. Does that address
> all the issues here?

Sorry it looks like I got my bugs confused. The comment stripping will resolve bug 801046. I don't think it fixes this specific bug since it is style attribute based. I'm not exactly sure how to fix this issue without checking the style values for disallowed input.

I'll look into that.
Whiteboard: [site:developer.mozilla.org][specification-like][type:bug] → [site:developer.mozilla.org][type:bug]
Assignee: eshepherd → lcrouch
Priority: P1 → P2
Adding keywords to bugs for metrics, no action required.  Sorry about bugmail spam.
Keywords: wsec-xss
Adding all MDN devs to cc list of these security bugs.
As of today, I can repro using IE9 compat mode on this page: https://developer.allizom.org/en-US/docs/User%3AIlovecheese

  <!--[if IE]>
  <meta http-equiv="imagetoolbar" content="no">
  <script src="/media/js/libs/html5.js"></script>
  <![endif]-->
:davidwalsh - can you take a look at this again?
Flags: needinfo?(dwalsh)
Just saw this, looking at it today.
Flags: needinfo?(dwalsh)
I just did the following:

1.  Downloaded IE9 VM from Microsoft
2.  Visited https://developer.allizom.org/en-US/docs/User%3AIlovecheese
3.  Viewed in both IE9 and IE9 Compat View, standard and quirks mode:  no alert
4.  Visited https://developer.allizom.org/en-US/docs/User%3AIlovecheese$edit
5.  Viewed in both IE9 and IE9 Compat View, standard and quirks mode:  no alert
6.  Switched between source and HTML mode:  no alert
7.  Added the <div.... to document, saved:  no alert

I cannot duplicate this issue
Rebecca: Can you try reproducing this again using whatever steps you used in comment #36?
Flags: needinfo?(rbillings)
I am no longer able to reproduce this issue on IE9 with or without the compatibility mode. I no longer have an IE9 VM so I tested this using Saucelabs- there were no alerts in multiple compat view modes nor in document modes.
Flags: needinfo?(rbillings)
Ok. I'm going to mark this as RESOLVED/FIXED, though I'm not sure what fixed it specifically.

If it's still a problem, please reopen.
Status: NEW → RESOLVED
Closed: 12 years ago → 8 years ago
Resolution: --- → FIXED
For bugs that are resolved, we remove the security flag. These haven't had their flag removed, so I'm removing it now.
Group: websites-security
You need to log in before you can comment on or make changes to this bug.