Investigate/Mitigate potential XSS risk in ExtensionsUI.jsm

RESOLVED WORKSFORME

Status

WebExtensions
General
RESOLVED WORKSFORME
a year ago
28 days ago

People

(Reporter: freddyb, Assigned: mstriemer)

Tracking

({sec-audit})

Firefox Tracking Flags

(Not tracked)

Details

Having looked at the code for a while I'm not particularly worried about this, but I wanted to track this to err on the side of caution.
If this turns out exploitable, this is a sec-critical issue. Please help me investigate and discuss possible mitigations.

ExtensionsUI.jsm shows permission prompts involving add-on names and add-on metadata that is potentially harmful, because it does so using innerHTML assignments.

The source line in question is at <http://searchfox.org/mozilla-central/rev/1a054419976437d0778a2b89be1b00207a744e94/browser/modules/ExtensionsUI.jsm#381>:

>  showPermissionsPrompt(browser, strings, icon, histkey) {
>    function eventCallback(topic) {
>      let doc = this.browser.ownerDocument;
>      if (topic == "shown") {
>        doc.getElementById("addon-webext-permissions-notification")
>           .description.innerHTML = strings.header;


Understanding that strings.headercontains HTML, it's unfeasible to escape special characters completely. An alternative would be to use a sanitizer like nsTreeSanitizer or to re-architect the code that the HTML will be created within showPermissionsPrompt() and the parameter is just text that can be properly escaped (or assigned via textContent).

I have assigned you, Mark, because you touched the relevant lines last. If you are not a good fit, please help me find someone else so the bug is not dropped.
The add-on name in this string is sanitized here:

http://searchfox.org/mozilla-central/rev/1a054419976437d0778a2b89be1b00207a744e94/browser/modules/ExtensionsUI.jsm#338

so I'm not quite sure what the issue is.
As mentioned in comment 0, I'd rather check in with you fine folks before drawing an incorrect conclusion. Filing a security bug is, in my experience, a better forum than starting an email thread, even if it turns out not be closed as WORKSFORME.
Thank you for taking a look, Kris.
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → WORKSFORME
Shouldn't we remove the s-s flag then?
Group: core-security

Updated

28 days ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.