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

Status: VERIFIED FIXED

(Firefox :: Security, defect)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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.

Comment 1

[:Gijs (he/him)]

Seriously? :-(

Comment 2

[:Gijs (he/him)]

Oh, awesome, this is inside a localized string so we can't fix it on beta in a straightforward way anymore.

Comment 3

[:Gijs (he/him)]

Attachment: 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.

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

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

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

Nice solution, lgtm.

Comment 5

[:Gijs (he/him)]

Attachment: Patch for Nightly

Comment 6

[:Gijs (he/him)]

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

Comment 7

[:Gijs (he/him)]

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...

Comment 8

[:Gijs (he/him)]

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).

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e91665dde88e

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7970459fa07d
https://hg.mozilla.org/releases/mozilla-beta/rev/055a4be6a0e7

Comment 11

[Petruta Horea [:phorea], Desktop QA]

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.