Clarify security implications

[ghost]

Some examples (eg inpage-toolbar-ui ) inject code into a webpage. I would assume if the webpage isn't under our control it can easily delete, obscure and otherwise manipulate the displayed iframe?

[rpl]

@rgh36167 good point

Follows an initial list of scenarios (all tried on the inpage-toolbar-ui example), which can be helpful to put together a similar list on MDN to help an add-on developer to evaluate the security implications of the features he is planning to use in its add-on.

The web page currently can:

• remove the iframe completely (and eventually inject another one)
• hide the iframe by applying styles to the iframe element
• update the src attribute of the iframe to a "data:" url or an "https:" url (if the remote url doesn't prevent its loading into an iframe)
• update the src attribute of the iframe with the same url with the changes on the hash and search component (no reloading happens, but the new hash and search component seems to be available in the iframe)

The web page currently is not able to:

• use iframe.contentWindow.eval to execute javascript code with the privileges and API of the inject (raise security errors)
• access the document loaded in the injected iframe and its DOM elements (raise security errors)
• update the src with an arbitrary "moz-extension:" url (security error and no loading happens in the iframe)

[ghost]

Discussion has been moved to:
https://bugzilla.mozilla.org/show_bug.cgi?id=1287590

[wbamberg]

Given https://bugzilla.mozilla.org/show_bug.cgi?id=1287590#c5, should we add a warning, or just remove this example?

(I tend to think we should remove this example.)

@rgh36167 , @rpl ?

[ghost]

People are doing it all the time anyway - me shudders that developers are trying to use this to implement password managers (https://bugzilla.mozilla.org/show_bug.cgi?id=792479#c61)

Leave it as prominent example howto not do it?

The code doing the automated review of AMO submissions should detect this and similar and give a warning.

[ghost]

Btw to ammend the list of possible exploits, I see nothing that would stop a hostile webpage from using HTML5 canvass to get "screenshots" of the iframe.

[rpl]

@wbamberg @rgh36167 I'm ok with removing this example so that it is not as tempting as it is currently.

At the same time, as @rgh36167 link above shows, add-on developer are currently digging into the Firefox sources to achieve this in Add-on SDK add-ons, that if I'm not wrong has a real toolbar ui component, using the require("chrome") trick, and so I'm not sure that it is not going to be used even without an example.

Nevertheless, I totally agree that this feature should not be used to implement UI of a password manager addon or any other security oriented features.

In general every part of an addon that is directly accessible to a webpage is going to provide a greater attack surface (e.g. when a content script exchanges messages with a webpage, how can the content script be sure that something in the page is not faking the real source? if we export a function from the content script into the webpage using the newly provided exportFunction helper, how can the content script be sure that the function is not used to inject fake information inside the extension?)

On the other hand, this is not worst of injecting single DOM elements into the page from a content script, it is only better isolated from the rest of the page, and I'm pretty sure that it can be helpful in the context of devtools addons.

Can a re-write of the example and its readme helpful? (so that it is more clearly suggested as a way to augment the page and not the extension/browser ui)
would it be more clear what is the scenario when this feature is actually suggested (and even more important, when this feature should not be used)?

[ghost]

@rpl : the SDK also has the panel high level API which would do what many people are trying to achieve with the toolbar ui, I was not even aware of the chrome trick you mention. If Firefox made the equivalent https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/browserAction/openPopup available for addons it would solve most of the problem.. only leaving the problem with extensions ported from the other browser.

The exportFunction (which doesn't seem available for webextensions?) could be in theory used to redefine all DOM element access function of the page to effectively hide and protect the iframe or other injected elements , this would also require catching/handling focus events, z-Index manipulations, canvas functions and perhaps a few more. This looks like a nightmare to get it right for the average addon auothor but if there is real demand to inject elements which are invisible/protected from the webpage mozilla could decide to implement some safe way to do it?

[rpl]

@rpl : the SDK also has the panel high level API which would do what many people are trying to achieve with the toolbar ui

@rgh36167 I know that the Addon SDK provides a proper high level API, and it seems interesting that, nevertheless, (and I'm wondering why) add-on developers are trying to inject iframes into the page from an Addon SDK add-ons.

browserAction.openPopup is a private API on Chrome and it looks that we are not going to implement it or make it available to any addon (The related bugzilla issue has been renamed from 'Implement browser.browserAction.openPopup' to 'Remove browser.browserAction.openPopup documentation', https://bugzilla.mozilla.org/show_bug.cgi?id=1278180).

The exportFunction helper is available in content scripts starting from Firefox 49 (Bug 1280482 - Give content scripts access to export helpers), I wasn't mentioning it as a way to manually refine the DOM elements to make then safer, but as another scenario where the add-on is exposing something to a webpage that, if not used properly, could provide a bigger attack surface that malicious code running in a webpage could try to take advantage of.

[ghost]

@rpl - I was also wondering why people would inject iframes in an unsafe and rather complicated way instead of using a clean safe easy high level api.. probably a part of it are developers porting their addons from Chrome who are not aware that a better and safer way to do it exists. Other cases are perhaps where addon writers want to "integrate" their content with the webpage which may or may not be dangerous depending on the situation.

browserAction.openPopup is a private API on Chrome and it looks that we are not going to implement it or make it available to any addon (The related bugzilla issue has been renamed from 'Implement browser.browserAction.openPopup' to 'Remove browser.browserAction.openPopup documentation', https://bugzilla.mozilla.org/show_bug.cgi?id=1278180).

That is very unfortunate, if there is no safe alternative to do it than WebExtensions would be a huge step backward for Addon development. I think this should be reopened or a new bug filled because afaics the decision to not implement it was done long before the ui-iframe-toolbar security concerns landed in bugzilla.

[Rob--W]

For those who aren't following https://bugzil.la/1278180 : Note that Firefox 57 implements openPopup for browserAction/pageAction, guarded behind a user gesture - see https://bugzil.la/1341126.

[ghost]

Do I understand it right that it would work or example from a listener added by window.addEventListener("touchstart", ....) ?

[Rob--W]

Do I understand it right that it would work or example from a listener added by window.addEventListener("touchstart", ....) ?

Yes, but only from an extension page (moz-extension://...).

[ghost]

@Rob--W: if so it would not help anything with the security issue - those were concerning code injected into potentially hostile webpages. Anything in moz-extension should be under our control anyway?

[Rob--W]

@rgh36167 I don't fully understand you. Are you saying that the feature is useless because it cannot be used from content scripts? If so, I agree and posted https://bugzil.la/1392624. If not, what do you mean?

[ghost]

@Rob--W not exactly useless as it can have other uses but otherwise agree.

[rebloor]

Closing as there's been no further comment on this issue for 6 years.