1842009 - storage.onChanged should not trigger for storageArea session in content scripts

Status: VERIFIED FIXED

(WebExtensions :: Storage, defect, P2)

Description

[Rob Wu [:robwu]]

Attachment: storage-session-onChanged.zip

The storage.session API as implemented in bug 1823713 is supposed to limit the visibility of the storage to the extension process. Content scripts are not supposed to see the data, especially because the opt-in through the setStorageAccess method has not been implemented (bug 1823717).

The implementation however exposes the data through the storage.onChanged event: https://searchfox.org/mozilla-central/rev/9a4666e63199bd1bcfc9095f6efec3488c358458/toolkit/components/extensions/parent/ext-storage.js#76,82-85,93

STR:

1. Load the attched add-on, e.g. at about:debugging.
2. Visit example.com
3. Click on the extension button. That will register storage.onChanged in the content script, and call storage.session.set, storage.local.set, storage.sync.set, and then display the received storage.onChanged events.
4. Look at the results.

Expected:

• The output should list "local" and "sync"

Actual:

• The output lists "session", "local" and "sync"

Note: extension developers affected by this: Use the StorageArea-specific onChanged listener instead of the global storage.onChanged listener to receive events for specific storage areas. That feature was introduced in Firefox 101 (bug 1758475).

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1823713

:zombie, since you are the author of the regressor, bug 1823713, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 2

[Donal Meehan [:dmeehan]]

There is already a pending needinfo on :zombie
:robwu could you provide some additional details around the impact. For example:

• We had one extension hit problems with Fx115 (Bug 1838448), do you expect that other extensions may hit this?
• Is this something we want to include a fix for in a Fx115 dot release?

Comment 3

[Rob Wu [:robwu]]

(In reply to Donal Meehan [:dmeehan] from comment #2)

There is already a pending needinfo on :zombie
:robwu could you provide some additional details around the impact. For example:

• We had one extension hit problems with Fx115 (Bug 1838448), do you expect that other extensions may hit this?

The reporter of that other bug confirmed that they weren't affected by this bug (https://bugzilla.mozilla.org/show_bug.cgi?id=1838448#c15). Other extensions that do could be affected. The API is relatively new and the API usage is relatively low.

• Is this something we want to include a fix for in a Fx115 dot release?

Yes. The feature is meant to be used to store potentially sensitive data of extensions, notably password managers, and we should not be sending this data to (untrusted) web content processes. Tom will fix this and request an uplift.

Comment 4

[Tomislav Jovanovic :zombie]

Attachment: Bug 1842009 - Don't send session area storage.onChanged events to content scripts r?robwu

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8af5adb5ae93
Don't send session area storage.onChanged events to content scripts r=robwu

Comment 6

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/8af5adb5ae93

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:zombie, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox116 to wontfix.

For more information, please visit BugBot documentation.

Comment 8

[Tomislav Jovanovic :zombie]

I'm going to ask for uplift to release 115 as well, in case there's a dot release planned.

Comment 9

[Tomislav Jovanovic :zombie]

Comment on attachment 9343266 [details]
Bug 1842009 - Don't send session area storage.onChanged events to content scripts r?robwu

Beta/Release Uplift Approval Request

• User impact if declined: We introduced an API in 115, with unwanted behavior that exposes storage.session events to content scripts. This new area is stored in memory, and often used for decrypted data, such as password manager's vaults. While this isn't a vulnerability in itself, it expands the potential attack surface if another part of the browser (or extension) is compromised.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): One additional condition in code that's well understood and not particularly complicated.
• String changes made/needed: none
• Is Android affected?: Yes

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Because of the long lifetime of ESR, the potential for an exploit to use this bug to compromise user's data is higher.
• User impact if declined: Same as above.
• Fix Landed on Version: 117
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Same as above, this code hasn't changes since 115.

Comment 10

[Dianna Smith [:diannaS]]

Comment on attachment 9343266 [details]
Bug 1842009 - Don't send session area storage.onChanged events to content scripts r?robwu

Approved for 116.0b5

Comment 11

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/614c297e95c2

Comment 12

[Dianna Smith [:diannaS]]

Comment on attachment 9343266 [details]
Bug 1842009 - Don't send session area storage.onChanged events to content scripts r?robwu

Approved for 115.1esr

Comment 13

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/dc7a19fb472e

Comment 14

[Alex Cornestean]

Verified as Fixed. Tested on the latest Nightly (117.0a1/20230713214846) and Beta (116.0b5/20230713175905) under Windows 10 x64 and macOS 11.3.1.

The output lists only “local” and “sync” and NOT “session”, confirming the fix. For more details, see the attached screenshot.

Once ESR 115.1 is available, I’ll verify there too.

Comment 15

[Alex Cornestean]

Attachment: 2023-07-14_09h22_06.png

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9343266 [details]
Bug 1842009 - Don't send session area storage.onChanged events to content scripts r?robwu

We aren't planning any more dot releases this cycle. This fix will ship in 116/115.1esr going out on August 1.

Comment 17

[Tomislav Jovanovic :zombie]

Sounds good, thanks Alex and Ryan.

Comment 18

[Alex Cornestean]

Verified as Fixed. Tested on the latest ESR (115.1.0esr/20230724124053) under Windows 10 x64 and Ubuntu 22.04 LTS.

The output lists only “local” and “sync” and NOT “session”, confirming the fix. For more details, see the attached screenshot.

Comment 19

[Alex Cornestean]

Attachment: 2023-08-01_10h46_49.png