executeScript API may inject scripts in an unexpected target at runtime

[Rob--W]

The chrome.tabs.executeScript API (and also Chrome's proposed chrome.scripting.executeScript API) have an inherent design flaw (TOCTOU) that is not present with statically registered content scripts.

The executeScript API looks like this:

chrome.tabs.executeScript(tabId, {
  frameId: ...,
  file: "contentscript.js",
});

For script execution at runtime by the extension, the typical pattern is that an extension calls executeScript in response to an event related to a tab or frame (e.g. a navigation, network request or user interaction with an extension button). The problem with the executeScript API is that it only takes a tabId and frameId as an identifier. These IDs uniquely identify a frame (BrowsingContext), but not the document within the frame. Consequently, it's possible for a page to have been navigated and for the extension to inadvertently run a content script on an unexpected website.

This issue is not present with content_scripts in manifest.json (or APIs that register scripts in a declarative way such as browser.contentScripts.register), because the browser engine checks the URL right before injection.

There are multiple ways to mitigate this issue with executeScript, e.g. any of the following:

• Allow or require extensions to specify the expected URL (or match pattern).
  • Ideally this parameter would be required to encourage extensions to be explicit about their desired target of the injection.
• Allow extensions to specify additional parameters with the injected content script, so they can compare the URL before running the script.
  • Extensions can already do this, but requires them to communicate the URL to the content script in some way (e.g. extension messaging, injecting dynamic code with arguments (not files) as content scripts).
  • This moves the responsibility of checking the URL to the extension. Extensions are not doing this already.
• Introduce the concept of an identifier for the document within a frame (tabId + frameId + documentId), and allow extensions to pass this to executeScript (and potentially any API where this is relevant, beyond content scripts, e.g. extension messaging APIs).
  • The disadvantage over the other suggestions is that the documentId may not always be known when an extension uses this.
  • The advantage over the other suggestions is that this allows extensions to uniquely identify documents, and not be confused by navigations without URL changes (e.g. page reloads) and URL changes without navigation (e.g. reference fragment changes or history.pushState).

[carlosjeurissen]

Proposal from Google on documentIds:
https://docs.google.com/document/d/1xOe5e8CDU9HM6sazGg6aqvK0iPnptilmwBS9qiZSfkY/edit

[oliverdunk]

A few quick thoughts post-meeting on the Chrome proposal…

I love the document ID concept as a solution for “time of use” problems. With the current APIs there are cases where these problems are unavoidable, and being able to target a specific document (and therefore a certain origin) is a really nice capability.

I do have some backwards compatibility concerns when it comes to pre-rendering:

• Do pre-rendered frames act normally from an extension’s perspective from when they are created to when they are destroyed? Or can they become temporarily unresponsive? The latter could cause backwards compatibility issues because extensions may not have retry behaviour if an important message to the tab fails.
• Some extensions may not be robust against a frame’s ID changing when the pre-rendered frame is promoted to the active, top-level one. This could have security implications if the tab with frame ID 0 is changing without the usual page navigation events firing.

While I realise it isn’t part of the proposal, someone at WECG suggested making this opt-in, so I wanted to provide some thoughts on that too. Unfortunately, I think that could cause even worse backwards compatibility issues. Unless all of the events are queued up until the page becomes active (onDOMContentLoaded etc.) a new page is magically going to become frame 0 without any of the events extensions are built to expect.

Finally, are there any plans to indicate to extensions when a pre-rendered frame becomes active? That’s important because an extension may want to (for example) update UI or clear data based on when the page the user is looking at changes.

Appreciate all of the work here. It would be great to come to a consensus because Safari does similar pre-rendering and I expect other browsers may want to experiment in the future.

[zombie]

I've left a few comments from the Firefox point of view in the doc, but the biggest issue seems to be that it's not obvious from the proposal that top frame in a pre-rendered tab would not have the value frameId == 0.

That would mean the value of the frameId could change during it's lifetime, which probably breaks many baked-in assumptions in a lot of code.

I expect a much more compatible change would be to keep it zero during pre-render, and let extensions distinguish if it's the currently "active" frame using documentLifecycle.