1699444 - Tabs opened by extensions are displayed as private when they are not

Status: RESOLVED FIXED

(Firefox for Android :: WebExtensions, defect, P2)

Description

[Wladimir Palant]

When the current tab is a private tab, a call to browser.tabs.create() will create a new private tab, at least as far as the Fenix UI is concerned. GeckoView does not consider these tabs to be private however: cookies are shared with non-private tabs, extensions without access to Private Browsing can access the tab and tab.incognito is reported as false.

Steps to reproduce:

1. Install any extension with tabs permission, e.g. Dark Reader. Do not allow in private browsing.
2. In a debugging session, inspect the extension and open the Console tab.
3. Run browser.tabs.onUpdated.addListener((tabId, changeInfo, tab) => console.log(tabId, changeInfo, tab)) to be notified about tab changes.
4. In the browser, create a private tab and load https://example.com/. Note that the debugging session doesn’t display the page loading (extension has no access to private browsing).
5. No go to the debugging session and run browser.tabs.create({url: "https://google.com/"})
6. Note how the debugging session displays updates while this page is loading. Inspecting cookies confirms that these are shared with the regular Google tab.
7. Go to Fenix and check the tabs – it claims that this tab is private, even though it clearly isn’t.

Note that a side-issue here is an extension without private browsing access creating a private tab, not something that should be possible IMHO. Also, note that an extension can detect the situation when a private tab is activated – tabs.getCurrent() will return undefined then. So the extension can open a link at the right time for to to be displayed as a “private” tab and trick the user.

Comment 1

[Christian Sadilek [:csadilek]]

This needs fixes in both Fenix and GV:

The logic in Fenix to make sure extension tabs are opened in the current browsing mode predates support for the "private browsing access flag" and still assumes that extensions are allowed to do that. We need to update this logic to not open private tabs when the extension isn't allowed to run in private browsing.

GV should likely throw an error here as Fenix does open a private gecko session but the extension tab ends up being not private. CCing Agi.

The risk here is mitigated as we currently only support extensions from the recommended extension program.

Comment 2

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Dan, is sec-moderate appropriate here since we only allow extensions from the allow list we control?

Comment 3

[Rob Wu [:robwu]]

There is no explicit control over the (private browsing) settings used to create a new tab. The information is not passed to GeckoView:WebExtension:NewTab, so GeckoView and later cannot really know which state to use.

This may be a consequence of the fact that in the extension API, private browsing access is tied to windows on desktop. On desktop a window is a collection of tabs, but despite there being one from the perspective of the user, this is not visible in the extension API. It is not possible to explicitly open a private browsing tab at the moment (bug 1372178).

Comment 4

[Chris Peterson [:cpeterson]]

Unassigning bugs assigned to Arturo

Comment 7

[Rob Wu [:robwu]]

This is relatively serious - despite the UI showing being in private browsing mode, the tab is in regular browsing mode. I'll try to work on a patch.

How this happens:

• The privateMode flag is NOT set in TabDelegate.onNewTab at https://searchfox.org/firefox-main/rev/d154a3eb431156b2bd63d3d5f96c82655b4e782d/mobile/android/android-components/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/webextension/GeckoWebExtension.kt#265
• But the tabHandler.onNewTab call a few lines below ends up calling onNewTabOverride in openTab (in WebExtensionSupport.kt - see bug 2040387 for a recent patch that fixes up the plumbing of flag propagation to see how it all flows through from then on).
• onNewTabOverride ends up deciding on their own how to set the private browsing flag, based on whatever the selected tab is at the time of the call: https://searchfox.org/firefox-main/rev/d154a3eb431156b2bd63d3d5f96c82655b4e782d/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/FenixApplication.kt#827-830,836

At the very least, the flag should be consistent. I'm going to see if I can fix it in an architecturally sound way, but if not, then at the very least we should be able to make sure that the flag is correct.

Comment 9

[Rob Wu [:robwu]]

I have a fix ready, and was looking for bugs to attach the fix to. Initially I considered attaching it to bug 2041306 because it is an innocent bug, and my fix addresses the issue reported there.

But I also found bug 1870978, filed two years ago, where the root cause is essentially this bug. That other bug has been resolved in the meantime by the feature switching away from extensions. But duplicates were linked to that bug. And https://bugzilla.mozilla.org/show_bug.cgi?id=1870978#c3 pretty much covers part of my analysis in comment 7 above.

P.S. I found that this issue was a regression from years ago by https://searchfox.org/firefox-main/commit/c099bd8bb802860f49b48ce24de02a0069d2fdcb . Someone reported at https://github.com/mozilla-mobile/fenix/issues/8153 that extension created tabs appeared in the wrong private browsing mode. The issue was resolved by styling the tab as a private browsing window, but unfortunately the content was not loaded in a private browsing window.

Comment 10

[Rob Wu [:robwu]]

I discussed this issue with dveditz, and considering that this is pretty much public knowledge (bug 1870978 and its duplicates) and not really exploitable, it serves the public more to know about the bug than to keep it hidden. I'll attaching patches shortly, I'm currently working on some integration tests to make sure that this does not regress.

Comment 11

[Rob Wu [:robwu]]

Attachment: Bug 1699444 - Plumbing for private flag of tabs.create()

This moves the private browsing specifier for tabs.create(),
from the Fenix layer to the Android-Components layer, very close to the
GeckoView layer. That paves the path for bug 1372178, eventually.

More importantly, this also fixes two functional defects:

• The original implementation in Fenix was only cosmetic (bug 1699444).
  Now the opened tab is really sharing state with private browsing tabs.

• The original implementation relied on an active tab, and would
  otherwise open a non-private tab (bug 2041306). The new implementation
  checks the app's current private browsing mode and is more reliable.

This patch includes some unit tests to test interaction with A-C, a full
end-to-end test involving Fenix will be added in bug 2054102.

Comment 12

[Rob Wu [:robwu]]

Attachment: Bug 1699444 - Reject tabs.create() call when PBM is not allowed

The tabs.create() call does currently not have a way for extensions to
specify where the tab should be opened (bug 1372178), so the
implementation currently picks an arbitrary tab. But this could enable
extensions without private browsing access to open tabs in private
browsing mode (PBM), so we are adding a check to guard against it.

An alternative implementation is to open the tab in the regular browsing
mode. But the user would not notice that, and it could be confusing if
the user returns to the non-private browsing mode and finds some
seemingly random tab. So for now we just reject tabs.create() calls.

Extensions hitting this would encounter "Cannot create new tab" from
https://searchfox.org/firefox-main/rev/a07b2cb8ff33387a81968a7ed9d59ebdc4dad607/mobile/shared/modules/geckoview/GeckoViewTab.sys.mjs#151

TestRunnerActivity does not recognize the concept of private browsing,
so this patch only has tests for the A-C layer, not a mochitest.
Bug 2054102 will add a more thorough integration test involving Fenix.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/156251d663b4
https://hg.mozilla.org/integration/autoland/rev/9133a64d43a7
Plumbing for private flag of tabs.create() r=geckoview-reviewers,android-reviewers,android-addons-reviewers,owlish,willdurand,twhite
https://github.com/mozilla-firefox/firefox/commit/07ee9c98e872
https://hg.mozilla.org/integration/autoland/rev/cb547f7ea0b5
Reject tabs.create() call when PBM is not allowed r=android-reviewers,android-addons-reviewers,willdurand,twhite

Comment 14

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/9133a64d43a7
https://hg.mozilla.org/mozilla-central/rev/cb547f7ea0b5

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Did you want to nominate this for the Fx155 relnotes? Possible wording:

On Firefox for Android, tabs opened by an extension are no longer shown as private browsing tabs when they are not actually private.

Comment 16

[Rob Wu [:robwu]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #15)

Did you want to nominate this for the Fx155 relnotes? Possible wording:

On Firefox for Android, tabs opened by an extension are no longer shown as private browsing tabs when they are not actually private.

That suggested wording could give the impression that tabs are now opened in non-private mode instead of private browsing mode.

But what actually happens is:

• tabs created by extensions (and also extension popups panels, via bug 1748526) are now opened and loaded in private browsing if the user is in private browsing mode (previously the tab was visually opened in private browsing mode, but the content was loaded in non-private browsing mode).
• we now block the opening of tabs in private browsing mode by extensions for which the "Run in Private Browsing" toggle is not on.

A potential "unwanted" impact of this is that the user may be "logged out" if the opened tab depended on cookies. But I'd consider that to be a desires feature, when opened in private browsing mode. So I don't expect users to need to know about this change. Any extension that relied on the old behavior relied on undocumented buggy behavior that was Android-only and not matching the expectations conveyed by the UI.

The bug being fixed has privacy implications. Tabs created by extensions whilst in private browsing mode shared the browser session with regular browsing mode. So cookies written by these sessions may still stay behind. But history was not persisted, so even if users wanted to, they would not be able to see the affected sites. And even if they did, the only way to recover from unwanted data stored in regular private browsing is by clearing all browsing data. That might be a bit too much for most?

The worst impact is users who met all of these conditions:

• they installed an extension that called browser.tabs.create() whilst in private browsing mode.
• the opened tab triggers a network request that stores data (network cache, cookies, etc)
• that website/URL is sensitive
• Firefox's app data is exfiltrated. This is a very high bar, and usually requires physical access to the device, unlocked.

Since the bug was not visible to users (which is why this issue remained unresolved for so long), and there is not much to do for most users, I'm inclined to not add end user release notes for this. But I can be convinced otherwise.

Comment 17

[Rob Wu [:robwu]]

A user (Shawn) reached out to me and let me know that they can confirm the fix to the issue from https://bugzilla.mozilla.org/show_bug.cgi?id=1870978#c11 along with:

• they did observe history entries
• as a user they would have liked a release note entry, so that they can decide for themselves whether to take action, such as clearing browser data.

I'll check my reproduction steps next week and think of a suggested release note. If you have suggested phrasing, feel free to sent them my way.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Hi Rob, how about this?

On Firefox for Android, tabs opened by extensions during private browsing now load in private browsing. Previously they appeared private while their content loaded in regular browsing, which could leave site data behind.

Comment 19

[Rob Wu [:robwu]]

That suggested release note looks good to me. Thanks!

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Added to the Fx155 relnotes.