about:home's search doesn't sanitize input and uses it for .innerHTML (CSS running, JavaScript on events)

VERIFIED FIXED in Firefox 42

Status

()

Firefox
Security
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: aryx, Assigned: Gijs)

Tracking

({csectype-priv-escalation, regression, sec-high})

Trunk
Firefox 44
csectype-priv-escalation, regression, sec-high
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox41 unaffected, firefox42+ verified, firefox43+ verified, firefox44 verified, firefox-esr38 unaffected)

Details

Attachments

(2 attachments)

Starting with Firefox 42.0b1 and up to Nightly, the input of the search on about:home gets concatenated and set as .innerHTML in the search popup line telling the user what gets search with which search engine ("Search for foo with:"). The length of the input is restricted to 256 characters.

It's possible to use <style> to hide UI elements and create new ones which replace them. Event listeners defined by attributes fire, e.g.
<b onmouseover='alert("Hello")'>Come here</b>

It might be a tad easier to convince users to paste something into that search box than to paste it into the location bar. No idea if it's exploitable. Feel free to lift the access restriction.

Updated

2 years ago
Flags: needinfo?(dolske)
Keywords: csectype-priv-escalation, sec-high
(Assignee)

Comment 1

2 years ago
Seriously? :-(
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Flags: needinfo?(dolske)
(Assignee)

Comment 2

2 years ago
Oh, awesome, this is inside a localized string so we can't fix it on beta in a straightforward way anymore.
(Assignee)

Comment 3

2 years ago
Created attachment 8665832 [details] [diff] [review]
Patch for beta + aurora

Plus some sand/cleanup. No need to empty the element if we're guaranteed to put in 1 of 2 things that take up the entire element.
Attachment #8665832 - Flags: review?(ttaubert)
Comment on attachment 8665832 [details] [diff] [review]
Patch for beta + aurora

Review of attachment 8665832 [details] [diff] [review]:
-----------------------------------------------------------------

Nice solution, lgtm.
Attachment #8665832 - Flags: review?(ttaubert) → review+
(Assignee)

Comment 5

2 years ago
Created attachment 8665835 [details] [diff] [review]
Patch for Nightly
Attachment #8665835 - Flags: review?(ttaubert)
Attachment #8665835 - Flags: review?(ttaubert) → review+
(Assignee)

Comment 6

2 years ago
Comment on attachment 8665832 [details] [diff] [review]
Patch for beta + aurora

Approval Request Comment
[Feature/regressing bug #]: regressed by bug 1185845
[User impact if declined]: sec-high sec bug
[Describe test coverage new/current, TreeHerder]: maybe/hopefully
[Risks and why]: low, JS-only change
[String/UUID change made/needed]: no - the nightly patch will use a string change, this one doesn't have it
Attachment #8665832 - Flags: approval-mozilla-beta?
Attachment #8665832 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 7

2 years ago
annnnd then *I* messed up, because I should have requested sec-approval before doing this:

https://hg.mozilla.org/integration/fx-team/rev/e91665dde88e

Sorry. :-(

The good news is this doesn't affect release, "only" beta and aurora...
(Assignee)

Comment 8

2 years ago
Comment on attachment 8665835 [details] [diff] [review]
Patch for Nightly

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Relatively easily - it's just a question of realizing the user input used to go through innerHTML and doesn't anymore post-patch

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
no, but see above.

Which older supported branches are affected by this flaw?
only beta and aurora (42 & 43)

If not all supported branches, which bug introduced the flaw?
bug 1185845

Do you have backports for the affected branches?
yes.

How likely is this patch to cause regressions; how much testing does it need?
unlikely - the code touched is new in 42 (beta).
Attachment #8665835 - Flags: sec-approval?
https://hg.mozilla.org/mozilla-central/rev/e91665dde88e
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox44: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Attachment #8665835 - Flags: sec-approval? → sec-approval+
Attachment #8665832 - Flags: approval-mozilla-beta?
Attachment #8665832 - Flags: approval-mozilla-beta+
Attachment #8665832 - Flags: approval-mozilla-aurora?
Attachment #8665832 - Flags: approval-mozilla-aurora+
status-firefox41: --- → unaffected
status-firefox42: --- → affected
status-firefox43: --- → affected
status-firefox-esr38: --- → unaffected
tracking-firefox42: --- → +
tracking-firefox43: --- → +
https://hg.mozilla.org/releases/mozilla-aurora/rev/7970459fa07d
https://hg.mozilla.org/releases/mozilla-beta/rev/055a4be6a0e7
status-firefox42: affected → fixed
status-firefox43: affected → fixed
Flags: qe-verify+
Reproduced with Firefox 42 beta 1 on both about:home and about:newtab pages.

Verified as fixed using Firefox 42 beta 2, latest Dev Edition 43.0a2 and latest Nightly 44.0a1 2015-09-30 under Win 7 64-bit, Ubuntu 14.04 32-bit and Mac OS X 10.
Status: RESOLVED → VERIFIED
status-firefox42: fixed → verified
status-firefox43: fixed → verified
status-firefox44: fixed → verified
Group: firefox-core-security → core-security-release
Blocks: 1185845
Group: core-security-release
Keywords: regression
You need to log in before you can comment on or make changes to this bug.