1430511 - (CVE-2018-5133) Stored XSS in chrome://browser/content/preferences/in-content/findInPage.js line 306

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect, P1)

Description

[FLR]

Attachment: storexss.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20180103231032

Steps to reproduce:

Adding the following to prefs.js mozilla user profile :

user_pref("app.support.baseURL", "\"></html:a><html:img src=\"nopath.nopath\" onerror=\"alert('Stored XSS');\"></html:img><html:a href=\"");

Will cause an alert dialog box to popup when a user access chrome://browser/content/preferences/in-content/preferences.xul and perform a search with no results.

Since prefs.js can be modified using a low privileged process (runonce key in HKCU registry for example) and that from the JavaScript context one can access the Standard JavaScript code modules, one can turn mozilla into a persistent malware. For example, using the OS.File module it is possible to implement a ransomware.

REMARK : The dialog box do not show up when no results are returned when preferences page is loaded using the about:preferences URI.


Actual results:

With the preference app.support.baseURL set to the above value and from chrome://browser/content/preferences/in-content/preferences.xul searching for "ewwe" lead to the result shown in the attach storedxss.png




Expected results:

Value of the app.support.baseURL preference should be sanitized before being inserted into the html content at line 307 of chrome://browser/content/preferences/in-content/findInPage.js.

Comment 1

[FLR]

> REMARK : The dialog box do not show up when no results are returned when preferences page is loaded using the 
> about:preferences URI.

It's possible to make it work in about:preferences. To do so the erreounous URL must have a valid scheme for example: https://nopath.nopath.

Comment 2

[:Gijs (he/him)]

Could someone sec-rate this, please? :-)

Comment 3

[:Gijs (he/him)]

Attachment: Patch v0.1

Comment 4

[:Gijs (he/him)]

Comment on attachment 8943076 [details] [diff] [review]
Patch v0.1

:flod, can you doublecheck this is OK and offer any ideas you might have in terms of how we would deal with uplift? Thanks!

Comment 5

[Al Billings [:abillings - ex-MoCo]]

This is moderate to low, I think. Calling it sec-moderate for now.

Comment 6

[Francesco Lodolo [:flod]]

Comment on attachment 8943076 [details] [diff] [review]
Patch v0.1

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

We can uplift this to 59 without any concern, the r+ is for that (59/60).

Uplifting to 58 would be a serious problem though: we're past sign-offs, to update localizations relman would need to manually update sign-off data in Ship It for builds, and repeat it for any dot release. 
Realistically, we would end up with English text in the middle of preferences for all languages, which is not OK.

So, if we want to fix this for 58, we need a different approach for that version that doesn't involve changing strings. Can we hard-code 'https://support.mozilla.org/1/firefox/%VERSION%/%OS%/%LOCALE%/' instead of reading it from prefs, only for that version?

Comment 7

[Daniel Veditz [:dveditz]]

(In reply to Francesco Lodolo [:flod] from comment #6)
> Can we hard-code [url] instead of reading it from prefs, only for that version?

This isn't a serious enough problem to warrant cramming it into 58 last minute. An attacker who can write into your pref files or profile directory can do a lot more damage than just hoping someone searches for a non-existent preference.

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8943076 [details] [diff] [review]
Patch v0.1

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

Can you do the same for https://searchfox.org/mozilla-central/source/browser/components/preferences/in-content/preferences.js#543-547 ?

Comment 9

[:Gijs (he/him)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #8)
> Comment on attachment 8943076 [details] [diff] [review]
> Patch v0.1
> 
> Review of attachment 8943076 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Can you do the same for
> https://searchfox.org/mozilla-central/source/browser/components/preferences/
> in-content/preferences.js#543-547 ?

Per IRC, split this out to https://bugzilla.mozilla.org/show_bug.cgi?id=1431428 .

Comment 10

[:Gijs (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/233ed3c7331a
https://hg.mozilla.org/integration/mozilla-inbound/rev/8aa90bdddd80

Comment 11

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

https://hg.mozilla.org/mozilla-central/rev/233ed3c7331a
https://hg.mozilla.org/mozilla-central/rev/8aa90bdddd80