942307 - modal_dialog.js HTML/JS injection via innerHTML

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Browser, defect)

Description

[Rob Fletcher [:omerta]]

apps/browser/js/modal_dialog.js is vulnerable to potential HTML/JS injection.
The following line takes the output of prompt.message and uses it with innerHTML
without an sanitization [1]:
129         elements.customPromptMessage.innerHTML = prompt.message;

The file sanitizes this is exact field in other case statements [2], indicating
this should be escaped as well.

[1] http://mxr.mozilla.org/gaia/source/apps/browser/js/modal_dialog.js#129
[2] http://mxr.mozilla.org/gaia/source/apps/browser/js/modal_dialog.js#108

Comment 1

[Matt Wobensmith [:mwobensmith][:matt:]]

This seems bad. Who generates these prompts? The JS executes in a privileged context, yes? 

Rating sec-high at the moment.

Comment 2

[Rob Fletcher [:omerta]]

This is a certified app, so the JS executes as part of a certified application.

Still determining if we can control 'prompt.message' but thought I'd update with some notes so far:
 The handleBrowserEvent inside browser/js/browser.js, if the evt.type is
 'mozbrowsershowmodalprompt', calls ModalDialog.handleEvent(evt, tab.id).

 inside modal_dialog.js, handleEvent then calls this.show(origin) where origin ==
 tab.id

 So we are esssentially calling md_show(tab.id). md_shop() grabs the event from
 this.currentEvents[tab.id][0] and assigns it to 'evt'.

 If evt.detail.promptType is 'custom-prompt', then evt.detail.message is assigned
 to innerHTML.

Comment 3

[Andrew McCreight (out of office until 8/21) [:mccr8]]

Is this something you could look at Dale?  Thanks.

Comment 4

[[:fabrice] Fabrice Desré]

The content for the prompt can come from any web page, since it's basically what you set in alert("hello world!");

Comment 5

[Dale Harvey (:daleharvey)]

I can take a look, its not the same data as in alert(), thats escaped and formatted seperately

Comment 6

[Dale Harvey (:daleharvey)]

I have been trying to find an exploit for this bug havent been able, its possible that a future browser dev could unknowingly use this api on unsafe user code, but for now most of the content comes from http://mxr.mozilla.org/mozilla-central/source/b2g/locales/en-US/chrome/overrides/appstrings.properties

The appstrings do use newlines, and to be on the safe side we should still escape and format that prompt however, PR on the way

Comment 7

[Dale Harvey (:daleharvey)]

Attachment: Escape and newline custom browser prompts

Comment 8

[[:fabrice] Fabrice Desré]

Comment on attachment 8396680 [details] [review]
Escape and newline custom browser prompts

Oh the irony to review on github security bugs... don't do that please.

Comment 9

[Dale Harvey (:daleharvey)]

Apologies, I figured since I couldnt find an exploit that it was ok, wont do in future

Green @ https://travis-ci.org/daleharvey/gaia/builds/21629056
Landed @ https://github.com/mozilla-b2g/gaia/commit/f8627656b0164cf7cfd435bcf08a11300cae29fb