1782620 - shutdown() may be skipped due to inaccurate existingAddon in AddonInstall's startInstall

Status: NEW

(Toolkit :: Add-ons Manager, defect, P3)

Description

[Rob Wu [:robwu]]

When a XPI is downloaded, the following happens:

1. this.existingAddon is set to the current version of the add-on, if any in downloadCompleted)
2. The user is prompted for permissions before proceeding with installation.

• Note that this prompt is not immediately shown if the install is opened in a background tab. It only appears on focus.

3. startInstall then uses this.existingAddon in the following locations: https://searchfox.org/mozilla-central/rev/c2a2bf5a49626d63c4c0a5be42b6a93838c1b595/toolkit/mozapps/extensions/internal/XPIInstall.jsm#1763-1768,1770,1774,1779,1782-1783,1813-1814,1817,1833,1878-1879

Because of the arbitrary time between start and installation (step 2), it is possible for this.existingAddon to be inaccurate when it's used in step 3. This is a TOCTOU issue.
All uses of this.existingAddon should be audited and fixed if needed. Below are two STR with different outcomes.

shutdown not called, Extension not cleaned up, Extension broken

The most significant (and relatively common) consequence of this issue is that shutdown() may not be called. Consequently, it is possible for two Extension instances of the same extension to be running. This violates the assumption behind the design of the BootstrapScope (in AOM) / Extension classes, and results in unexpected errors.

The cause of the skipped shutdown is because the wrong branch is taken at https://searchfox.org/mozilla-central/rev/c2a2bf5a49626d63c4c0a5be42b6a93838c1b595/toolkit/mozapps/extensions/internal/XPIInstall.jsm#1878-1883,1889-1890:

        } else if (this.existingAddon) { // <-- should use this
          await lazy.XPIInternal.BootstrapScope.get(this.existingAddon).update(
            this.addon,
            !this.addon.disabled,
            install
          );
...
        } else { // <-- but got this instead because this.existingAddon was outdated/void
          await install();
          await lazy.XPIInternal.BootstrapScope.get(this.addon).install(

Below is an example, which shows that shutdown is not called (so the Extension's "shutdown" event is not triggered, and therefore the ExtensionAPI's onShutdown method isn't called). In the pageAction's case, this results in a stale page action button (for the old extension instance) and failure to register the button for the new version (because only one button may be registered at a time), which interrupts the full startup.

STR:

1. Visit an add-on listing, e.g. https://addons.mozilla.org/firefox/addon/crxviewer/ (I've chosen this add-on because it has a page_action button, which triggers a very visible error, see below).
2. Right-click on the install button (which is a link to a XPI file) and open it in a new (background) tab.

• Note: This creates an AddonInstall instance with existingAddon = null

3. Click on the install button to actually install the add-on.
4. Click on the tab from step 2, and confirm the installation.

• Note: existingAddon is still null while it should actually be pointing to the add-on from step 3.

5. Look at the global browser console.

Expected:

• No errors or weird messages

Actual:

• Error in PageActions.jsm:212: Action with ID 'crxviewer-rob_robwu_nl' already added
• Exception running bootstrap method startup on [e-mail]: Error: Action with ID 'crxviewer-rob_robwu_nl' already added(resource:///modules/PageAction.jsm:212:13) JS Stack trace: ... [e-mail]:129:44
  • Note that this shows that the onManifestEntry method was interrupted, which breaks startup (and deactivates the WebExtensionPolicy) which leaves the Extension in an inconsistent state until the extension is actually unloaded (e.g. a browser restart or disabled via about:addons).
• Error in ConduitsParent.jsm:165: TypeError: can't access property "remoteType", princ.addonPolicy.extension is undefined
• Error in ConduitsParent.jsm:438: Error: Unknown sender or wrong actor for recvCreateProxyContext (also for recvAPICall, recvAddListener, multiple of them).

Unable to install an add-on immediately after uninstalling one.

While the previous case is the most realistic scenario, here is another one for completeness.

STR:

1. Visit an add-on listing.
2. Install the add-on.
3. Right-click the button, and open the XPI in a new (background) tab.

• Note: existingAddon now points to the add-on from step 2.

4. Uninstall the add-on (e.g. by clicking the "Remove" button on AMO).
5. Switch to the tab from step 3, confirm installation.
   • Note: existingAddon still points to the add-on from step 2, which has already been removed. It should be pointing to nothing instead.
6. Look at about:addons
7. Look at the global browser console.

Expected:

• Add-on should install without errors or weird messages.

Actual:

• Add-on is not installed (despite AMO showing the "Remove" button, which suggests that the add-on should have been installed)
• Browser console shows Unexpected missing XPI state for add-on [e-mail]