Closed
Bug 873963
Opened 12 years ago
Closed 11 years ago
Arbitrary code execution from Font Inspector
Categories
(DevTools :: Inspector, defect)
DevTools
Inspector
Tracking
(firefox21 unaffected, firefox22+ verified, firefox23+ verified, firefox24+ verified, firefox-esr17 unaffected, firefox-esr24 fixed)
VERIFIED
FIXED
Tracking | Status | |
---|---|---|
firefox21 | --- | unaffected |
firefox22 | + | verified |
firefox23 | + | verified |
firefox24 | + | verified |
firefox-esr17 | --- | unaffected |
firefox-esr24 | --- | fixed |
People
(Reporter: marius.mlynski, Assigned: jwalker)
References
Details
(4 keywords, Whiteboard: [fixed-in-fx-team][adv-main22-])
Attachments
(1 file, 2 obsolete files)
2.81 KB,
patch
|
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
It is possible to execute arbitrary code when a user opens the Font Inspector on a malicious website containing a specially crafted font definition. The problem lies in the function |FI_buildPreview|, which appends user-controlled, unsanitized font-family names to a string containing CSS code. This allows code injection in the context of a document inheriting chrome privileges of chrome://browser/content/devtools/fontinspector/font-inspector.xhtml
Reporter | ||
Comment 1•12 years ago
|
||
This opens C:\Windows\System32\calc.exe on Windows or alerts Components.classes on other systems.
Updated•12 years ago
|
Keywords: sec-critical
Comment 2•12 years ago
|
||
Paul is apparently offline for a while, so assigning to jwalker per blame.
Assignee: nobody → jwalker
status-firefox21:
--- → unaffected
status-firefox22:
--- → affected
status-firefox23:
--- → affected
tracking-firefox22:
--- → ?
tracking-firefox23:
--- → ?
tracking-firefox24:
--- → ?
Updated•12 years ago
|
Assignee | ||
Comment 3•12 years ago
|
||
First stab at a fix. At least the exploit as presented doesn't work for me in my testing with this fix in place. Outstanding: Check this doesn't break anything, and check that there isn't an additional vector via the 'cssCode' parameter.
Assignee | ||
Comment 4•12 years ago
|
||
Update: This fix doesn't break any fontinspector unit tests. It looks like the cssCode argument is either "" (no risk) or it comes from cssText already parsed by the CSS engine (line 144), so at first pass attachment 752119 [details] [diff] [review] looks like it might work.
Assignee | ||
Updated•12 years ago
|
Attachment #752119 -
Flags: review?(dcamp)
Attachment #752119 -
Flags: feedback?(mgoodwin)
Assignee | ||
Comment 5•12 years ago
|
||
I'd be interested in some submerge advice; When do we need to submerge security patches?
Comment 6•12 years ago
|
||
Comment on attachment 752119 [details] [diff] [review] v1 r+ as a peer, but mgoodwin's review should be binding here.
Attachment #752119 -
Flags: review?(dcamp) → review+
Comment 7•12 years ago
|
||
I think we should not have allow-same-origin on that iframe sandbox attribute in font-inspector.xhtml - can we fix that too, please?
Comment 8•12 years ago
|
||
Comment on attachment 752119 [details] [diff] [review] v1 This looks good but I'd like to see the sandbox change too.
Attachment #752119 -
Flags: feedback?(mgoodwin) → feedback-
Assignee | ||
Comment 9•12 years ago
|
||
Attachment #752119 -
Attachment is obsolete: true
Attachment #752379 -
Flags: feedback?(mgoodwin)
Comment 10•12 years ago
|
||
Comment on attachment 752379 [details] [diff] [review] v2 Thank you.
Attachment #752379 -
Flags: feedback?(mgoodwin) → feedback+
Assignee | ||
Comment 11•12 years ago
|
||
Dan - can you give me some advice on landing security patches? The exploit for this bug involves a web page, and coercing the victim to open developer tools, change to the inspector, and open the font inspector. When should we submerge them with another patch? Do we need to take care over try too? Thanks.
Comment 12•11 years ago
|
||
You'll want to request sec-approval (and answer the auto-filled questions). I don't think you need to worry about "masking" this landing by merging with some other change (current release isn't affected), but we will want to coordinate landing on beta/aurora (the sec-approval process does that).
Assignee | ||
Comment 13•11 years ago
|
||
Comment on attachment 752379 [details] [diff] [review] v2 [Security approval request comment] How easily could an exploit be constructed based on the patch? * Fairly easily Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? * Commit comment does Which older supported branches are affected by this flaw? * aurora and beta, not release If not all supported branches, which bug introduced the flaw? * bug 697983 Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? * Unsure, but think there are no backports. Should not be too different or hard. How likely is this patch to cause regressions; how much testing does it need? * Unlikely
Attachment #752379 -
Flags: sec-approval?
Comment 14•11 years ago
|
||
Comment on attachment 752379 [details] [diff] [review] v2 sec-approval for m-c. Please don't make the commit message glaringly obvious. :-) Also, as soon as this gets into m-c, please prepare and nominate patches for Aurora and Beta so we can avoid shipping this issue to the public.
Attachment #752379 -
Flags: sec-approval? → sec-approval+
Assignee | ||
Comment 15•11 years ago
|
||
https://tbpl.mozilla.org/?tree=Fx-Team&rev=dea12c804f30 https://hg.mozilla.org/integration/fx-team/rev/dea12c804f30
Whiteboard: [fixed-in-fx-team]
Assignee | ||
Comment 17•11 years ago
|
||
[Approval Request Comment] Bug caused by (feature/regressing bug #): bug 697983 (font inspector) User impact if declined: Security risk Testing completed (on m-c, etc.): baked on on m-c for a week Risk to taking this patch (and alternatives if risky): low, that we cause regressions String or IDL/UUID changes made by this patch: none
Attachment #752379 -
Attachment is obsolete: true
Attachment #755410 -
Flags: approval-mozilla-beta?
Attachment #755410 -
Flags: approval-mozilla-aurora?
Updated•11 years ago
|
Attachment #755410 -
Flags: approval-mozilla-beta?
Attachment #755410 -
Flags: approval-mozilla-beta+
Attachment #755410 -
Flags: approval-mozilla-aurora?
Attachment #755410 -
Flags: approval-mozilla-aurora+
Comment 18•11 years ago
|
||
(In reply to Joe Walker [:jwalker] from comment #17) > Risk to taking this patch (and alternatives if risky): low, that we cause > regressions Not a very useful answer :) "low, only has the potential to break the font-inspector tool, no alternatives" is probably a better one.
Assignee | ||
Updated•11 years ago
|
Assignee | ||
Comment 19•11 years ago
|
||
Aurora landing: https://tbpl.mozilla.org/?tree=Mozilla-Aurora&rev=1702db2af514 https://hg.mozilla.org/releases/mozilla-aurora/rev/1702db2af514 Beta landing: https://tbpl.mozilla.org/?tree=Mozilla-Beta&rev=60c7f8e6ccd7 https://hg.mozilla.org/releases/mozilla-beta/rev/60c7f8e6ccd7
Assignee | ||
Updated•11 years ago
|
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 20•11 years ago
|
||
Verified fixed with Firefox 22 beta 4 (build ID: 20130605070403) on: Windows 7 64bit, Ubuntu 12.10 32bit and Mac OSX 10.8.3
QA Contact: manuela.muntean
Updated•11 years ago
|
status-firefox-esr17:
--- → unaffected
Updated•11 years ago
|
Whiteboard: [fixed-in-fx-team] → [fixed-in-fx-team][adv-main22-]
Comment 21•11 years ago
|
||
Verified fixed with Firefox 23 beta 1 (build ID: 20130625125232) on: Windows 7 64bit, Ubuntu 12.10 32bit and Mac OSX 10.8.3
Updated•11 years ago
|
Flags: sec-bounty?
Updated•11 years ago
|
Flags: sec-bounty? → sec-bounty+
Updated•11 years ago
|
status-firefox24:
--- → fixed
Comment 23•11 years ago
|
||
I wanted to verify this bug with Firefox 24 beta 6, but the exploit attached in comment 1 isn't available anymore, so I wasn't able to test the fix. Does anyone have any thoughts/suggestions?
Flags: needinfo?
Updated•11 years ago
|
Updated•11 years ago
|
status-b2g18:
--- → unaffected
Comment 27•10 years ago
|
||
Can the testcase be made public now? (Does it demonstrate anything that's not publicly known and not fixed?)
Flags: needinfo?(jwalker)
Assignee | ||
Comment 28•10 years ago
|
||
(In reply to Jesse Ruderman from comment #27) > Can the testcase be made public now? (Does it demonstrate anything that's > not publicly known and not fixed?) Hard for me to say for sure because I can't read attachment 751590 [details] any more. But, can't think of a reason to keep it non-public.
Flags: needinfo?(jwalker)
Updated•6 years ago
|
Product: Firefox → DevTools
Updated•3 months ago
|
status-b2g18:
unaffected → ---
Keywords: reporter-external
You need to log in
before you can comment on or make changes to this bug.
Description
•