Closed
Bug 848031
Opened 11 years ago
Closed 11 years ago
about:healthreport performs .innerHTML with unescaped JSON
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 22
Tracking | Status | |
---|---|---|
firefox20 | --- | unaffected |
firefox21 | --- | fixed |
firefox22 | --- | fixed |
firefox-esr17 | --- | unaffected |
b2g18 | --- | unaffected |
b2g-v1.1hd | --- | unaffected |
b2g-v1.2 | --- | unaffected |
People
(Reporter: gps, Assigned: gps)
Details
(Keywords: sec-high, Whiteboard: [fhr])
Attachments
(1 file)
941 bytes,
patch
|
Gavin
:
review+
bajaj
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
https://hg.mozilla.org/mozilla-central/file/c95439870e05/browser/base/content/abouthealthreport/abouthealth.js#l63 performs a |.innerHTML = | with a value that may contain unescaped HTML-like strings. e.g. if the JSON contains a string with "<", the produced HTML will be invalid and the feature won't work. This is theoretically an attack vector. If untrusted content could result in an arbitrary string being added to the FHR payload, that string could eventually make its way into the DOM of a chrome-privileged page and code execution could ensue. I don't believe untrusted content can currently cause injection of arbitrary strings into the FHR payload. But, this may not always hold. We should keep this door closed. Even if there isn't a security issue here, the issue of 'payloads containing "<" fail to render' is a legitimate bug that needs fixed and preferably uplifted to Aurora.
Assignee | ||
Comment 1•11 years ago
|
||
I /think/ this will do it. raw-data is a leaf node, so textContent should be equivalent to innerText. But I'm not a DOM expert.
Updated•11 years ago
|
Attachment #721347 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 2•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e67704d6b18
Target Milestone: --- → Firefox 22
Assignee | ||
Updated•11 years ago
|
Assignee | ||
Comment 3•11 years ago
|
||
Comment on attachment 721347 [details] [diff] [review] Don't use innerHTML on untrusted data, v1 [Approval Request Comment] Bug caused by (feature/regressing bug #): about:healthreport landing User impact if declined: possible injection of remote code into chrome compartment Testing completed (on m-c, etc.): Trivial DOM change. Manually verified fix. Risk to taking this patch (and alternatives if risky): Not much risk. String or UUID changes made by this patch: None.
Attachment #721347 -
Flags: approval-mozilla-aurora?
Comment 4•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0e67704d6b18 Can we get a test for this?
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Assignee | ||
Updated•11 years ago
|
Flags: in-testsuite? → in-testsuite-
Assignee | ||
Comment 5•11 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #4) > https://hg.mozilla.org/mozilla-central/rev/0e67704d6b18 > > Can we get a test for this? I have a bug open somewhere to track test coverage of FHR's browser components. Can't find it though... Parts of this page are due to be rewritten shortly. Hopefully tests will be part of the refactor.
Updated•11 years ago
|
status-firefox-esr17:
--- → unaffected
Updated•11 years ago
|
Attachment #721347 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•11 years ago
|
Group: core-security
Updated•11 years ago
|
status-b2g18:
--- → unaffected
status-b2g-v1.1hd:
--- → unaffected
status-b2g-v1.2:
--- → unaffected
You need to log in
before you can comment on or make changes to this bug.
Description
•