1514073 - Flash plugin can not save settings

Status: VERIFIED FIXED

(Core :: Security: Process Sandboxing, defect, P2)

Description

[Matthias Versen [:Matti]]

Firefox 66 nightly on windows10 and C:\WINDOWS\system32\Macromed\Flash\NPSWF64_32_0_0_101.dll


While investigating bug 1513966 I discovered that the Flash plugin can not save it settings anymore

> 9:33.11 INFO: No more inbound revisions, bisection finished.
> 9:33.11 INFO: Last good revision: ba708fde30b8f8d2ae4d1febfa2e88e2c1bf1cc4
> 9:33.11 INFO: First bad revision: a910482f4598931944910a357431b22f823578fb
> 9:33.11 INFO: Pushlog:
>https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ba708fde30b8f8d2ae4d1febfa2e88e2c1bf1cc4&tochange=a910482f4598931944910a357431b22f823578fb


STR:
1) load https://www.macromedia.com/support/documentation/en/flashplayer/help/settings_manager03.html
2) change for example the global storage setting to 10MB
3) switch the tab inside the flash settings area (example: Privacy settings) and switch back to the global storage tab
4) the changed setting from step 2 is back to it's original setting.

setting dom.ipc.plugins.sandbox-level.flash to "0" followed by a restart fixes the problem.

Comment 1

[David Parks [:handyman]]

This may or may not be the SharedObject thing from bug 1513966.  I've been looking into the issue from that end.  With luck, that will resolve this.

Comment 2

[Jim Mathies [:jimm]]

[Tracking Requested - why for this release]:
We're not going to get a fix for 66, so 67 is the new target.

Comment 5

[David Parks [:handyman]]

I duped bug 1513966 to this issue. This fix for this (coming soon) also fixes that issue. STR that demo the case in bug 1513966:

1. Go to https://www.permadi.com/tutorial/flashSharedObject/index.html
2. Scroll down to the applets and select one to click-to-play. Permit Flash to run.
3. Example 3 shows : "Number of times you have visited this page: 1". Reload the page.

Expected results:
Example 3 shows : "Number of times you have visited this page: 2"

Actual results:
Example 3 shows : "Number of times you have visited this page: 1" again.

Comment 6

[David Parks [:handyman]]

Attachment: Bug 1514073: Allow Windows plugin process to access Macromedia roaming folder (r?bobowen)

The sandbox already permits the process to create/delete the folder and access files in it. This patch gives is access to the folder itself, namely it allows NtQueryAttributesFile to evaluate it. For complex reasons, this fixes Flash's ability to store local objects (see AS3's SharedObject API).

Comment 7

[David Parks [:handyman]]

Tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3d19eed967853b1914212db04792bd2fb5db0447

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6868dec34052
Allow Windows plugin process to access Macromedia roaming folder (r=bobowen)

Comment 9

[Jim Mathies [:jimm]]

The fix here is pretty minimal and safe, so an uplift to 66 is possible.

Comment 10

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/6868dec34052

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

jimm, can you request uplift since it looks like David is on PTO?
Andrei, can your team verify the fix? Thanks!

Comment 12

[Hani Yacoub, Desktop QA]

I can confirm that the issue isn't reproducible any more on Windows 10 x64 and on Ubuntu 16.04 x64 on the latest Firefox Nightly 67.0a1 (2019-02-05).

But on Mac OS X 10.14 I faced the same issues mentioned in the user story and in comment 5.
Is this expected?
Thanks.

Comment 13

[Jim Mathies [:jimm]]

Haik, maybe we need a similar fix on OSX?

Comment 14

[Jim Mathies [:jimm]]

Comment on attachment 9040600 [details]
Bug 1514073: Allow Windows plugin process to access Macromedia roaming folder (r?bobowen)

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

sandbox work

User impact if declined

flash settings not saved

Is this code covered by automated tests?

No

Has the fix been verified in Nightly?

Yes

Needs manual test from QE?

No

If yes, steps to reproduce

List of other uplifts needed

Bug 1514073

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

adding a directory access exclusion, low risk change.

String changes made/needed

Comment 15

[Haik Aftandilian [:haik]]

(In reply to Jim Mathies [:jimm] from comment #13)

Haik, maybe we need a similar fix on OSX?

I've filed bug 1525625 to address this.

Comment 16

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9040600 [details]
Bug 1514073: Allow Windows plugin process to access Macromedia roaming folder (r?bobowen)

Fix for sandboxing Flash settings on Windows, OK to uplift for beta 6.

[Triage Comment]

Comment 17

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/releases/mozilla-beta/rev/9d24a22bcd5d

Comment 19

[Hani Yacoub, Desktop QA]

Since bug 1525625 was logged for the issue on Mac, I'll mark this as verified as fixed on Windows 10 x64 and on Ubuntu 16.04 x64.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Jim, this looks like a pretty safe patch. Is this something we might want to include as a ride-along for 65.0.1?

Comment 21

[Haik Aftandilian [:haik]]

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

Jim, this looks like a pretty safe patch. Is this something we might want to include as a ride-along for 65.0.1?

@Ryan, we have the same problem on Mac (bug 1525625) and the fix will probably also be low risk. I think the Mac fix will land within the next few days. If the timing lines up, it would be nice to fix this on both platforms.

Comment 22

[Hani Yacoub, Desktop QA]

Verified as fixed on Firefox 66.0b6 on Windows 10 x64 and on Ubuntu 16.04 x64.

Comment 23

[Jim Mathies [:jimm]]

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

Jim, this looks like a pretty safe patch. Is this something we might want to
include as a ride-along for 65.0.1?

Sure, that seems ok.