Closed
Bug 1087801
Opened 8 years ago
Closed 8 years ago
Some properties of the CSS object are not safe in a sandbox
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
(Keywords: regression, sec-moderate, Whiteboard: [adv-main34-])
Attachments
(1 file)
2.55 KB,
patch
|
bholley
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
dveditz
:
sec-approval+
|
Details | Diff | Splinter Review |
Specifically the ones that call GetParsingInfo: that assumes the global is a Window.
![]() |
Assignee | |
Comment 1•8 years ago
|
||
Attachment #8509965 -
Flags: review?(bobbyholley)
![]() |
Assignee | |
Updated•8 years ago
|
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Comment 2•8 years ago
|
||
Comment on attachment 8509965 [details] [diff] [review] Don't assume the global is a Window in the DOM CSS object Review of attachment 8509965 [details] [diff] [review]: ----------------------------------------------------------------- Oh, yuck - this is exactly the kind of stuff I'm worried about. Any ideas on how to better avoid this in the future? Just audit more carefully?
Attachment #8509965 -
Flags: review?(bobbyholley) → review+
![]() |
Assignee | |
Comment 3•8 years ago
|
||
> Any ideas on how to better avoid this in the future?
Not yet. :(
Do you think this is worth backporting anywhere?
Flags: needinfo?(bobbyholley)
Comment 4•8 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #3) > Do you think this is worth backporting anywhere? Yeah, until someone audits the devtools code that uses this we should assume the worst.
Flags: needinfo?(bobbyholley)
Keywords: sec-high
Comment 5•8 years ago
|
||
Comment on attachment 8509965 [details] [diff] [review] Don't assume the global is a Window in the DOM CSS object [Security approval request comment] How easily could an exploit be constructed based on the patch? Probably hard. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? It's clear that we were casting when we shouldn't be, but not much beyond that. Which older supported branches are affected by this flaw? FF32 and onward. If not all supported branches, which bug introduced the flaw? bug 1002280. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? trivial. How likely is this patch to cause regressions; how much testing does it need? Low risk.
Attachment #8509965 -
Flags: sec-approval?
![]() |
Assignee | |
Comment 6•8 years ago
|
||
The devtools code only uses CSS.escape(), which is not affected here. But yes, pretty happy to backport; it should just apply as-is.
Comment 7•8 years ago
|
||
Do we use CSS.supports() anywhere? I only found it in some tests so maybe sec-moderate, as a potential but unrealized risk, is more appropriate. sec-approval+ for landing now and on Aurora. If it's really sec-high we should land on beta too.
status-b2g-v1.4:
--- → unaffected
status-b2g-v2.0:
--- → affected
status-b2g-v2.0M:
--- → affected
status-b2g-v2.1:
--- → affected
status-b2g-v2.2:
--- → affected
status-firefox33:
--- → wontfix
status-firefox34:
--- → affected
status-firefox35:
--- → affected
status-firefox36:
--- → affected
status-firefox-esr31:
--- → unaffected
tracking-firefox35:
--- → +
tracking-firefox36:
--- → +
Keywords: regression
Updated•8 years ago
|
Attachment #8509965 -
Flags: sec-approval? → sec-approval+
![]() |
Assignee | |
Comment 8•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/51aa419a7b82
Target Milestone: --- → mozilla36
![]() |
Assignee | |
Comment 9•8 years ago
|
||
Comment on attachment 8509965 [details] [diff] [review] Don't assume the global is a Window in the DOM CSS object Approval Request Comment [Feature/regressing bug #]: Bug 1002280 [User impact if declined]: Probably none, but possible security issues. [Describe test coverage new/current, TBPL]: There isn't really any for this case. [Risks and why]: Very low risk. Just makes a case that used to crash throw instead. [String/UUID change made/needed]: None. I think we should go ahead and fix on beta too, so addons don't trigger this.
Attachment #8509965 -
Flags: approval-mozilla-beta?
Attachment #8509965 -
Flags: approval-mozilla-aurora?
Comment 10•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/51aa419a7b82
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Updated•8 years ago
|
Attachment #8509965 -
Flags: approval-mozilla-beta?
Attachment #8509965 -
Flags: approval-mozilla-beta+
Attachment #8509965 -
Flags: approval-mozilla-aurora?
Attachment #8509965 -
Flags: approval-mozilla-aurora+
Comment 11•8 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/e6fa5ecd3af2 https://hg.mozilla.org/releases/mozilla-beta/rev/ac59c74b9386
Comment 12•8 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/ac59c74b9386 https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/82a6ed695964 https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/82a6ed695964
Updated•8 years ago
|
Whiteboard: [adv-main34+]
Updated•8 years ago
|
Alias: CVE-2014-8633
Comment 13•8 years ago
|
||
Lowering severity to moderate: there are no uses of CSS.supports() in our tree (not counting unit tests for the feature). It requires Chrome privs to combine this with evalInSandbox() so this isn't an attack available to remote web content as implied by a "high" rating. It's possible there's an add-on out there using this, and for that add-on's users the danger may be "high" but overall the risk is lower.
Keywords: sec-high → sec-moderate
Updated•8 years ago
|
Alias: CVE-2014-8633
Whiteboard: [adv-main34+] → [adv-main34-]
Updated•7 years ago
|
Group: core-security → core-security-release
Updated•6 years ago
|
Group: core-security-release
Updated•4 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•