1475722 - Mac Flash sandbox causes empty file upload dialogs on OS X 10.9, 10.10

Status: VERIFIED FIXED

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

Description

[Haik Aftandilian [:haik]]

On OS X 10.9 and 10.10, with the latest Nightly which has the Mac Flash sandbox enabled, attempting to upload a file using a demo uploader[1] results in an empty file dialog. The file dialog side bar is usable, but all directories are displayed as empty.

This will also affect Beta builds that have Flash sandbox level set to 1 or above via pref "dom.ipc.plugins.sandbox-level.flash". Some early builds of Beta have the Flash sandbox on by default.

Release builds are not affected.

Workaround:
In about:config, set the preference "dom.ipc.plugins.sandbox-level.flash" to 0 and restart the browser. If the pref is not present, the sandbox is not enabled.

1. http://www.tinywebgallery.com/en/tfu/web_demo1.php

Comment 1

[Haik Aftandilian [:haik]]

I haven't been able to get the file dialogs working from the Flash plugin using our current sandbox level 1 which depends on sandbox exceptions triggered by file dialog activity.

The fix will be to allow blanket read access from the Flash sandbox on 10.9 and 10.10. This will become level 1 and the Flash sandbox that restricts read access will become level 2.

Comment 2

[Haik Aftandilian [:haik]]

Attachment: Bug 1475722 - Mac Flash sandbox causes empty file upload dialogs on OS X 10.9, 10.10

On 10.9 and 10.10, grant global read access to the Flash sandbox.

Change Flash sandbox levels by adding a new level 1 that includes
global read access which will be the default on 10.9/10.10.
Level 2 is the new default for 10.11 and above with file read
access enabled by file dialog activity.

Review commit: https://reviewboard.mozilla.org/r/257874/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/257874/

Comment 3

[Alex Gaynor [:Alex_Gaynor]]

Comment on attachment 8993063 [details]
Bug 1475722 - Mac Flash sandbox causes empty file upload dialogs on OS X 10.9, 10.10

https://reviewboard.mozilla.org/r/257874/#review265086

Would it be simpler to just structure this as `(if (< min-os-version 10) (allow file-read*))` in the policy, instead of using levels and prefs?

Comment 4

[Haik Aftandilian [:haik]]

Comment on attachment 8993063 [details]
Bug 1475722 - Mac Flash sandbox causes empty file upload dialogs on OS X 10.9, 10.10

https://reviewboard.mozilla.org/r/257874/#review265086

Yes, but using prefs makes it easy to tweak later after it's been released to the field if we encounter problems. From what I understand, we could ship an updated pref without requiring a full update. For example, if an updated version of Flash breaks this on 10.11 also, or we have to downgrade all OS versions to use level 1, we only need a pref update.

Comment 5

[Alex Gaynor [:Alex_Gaynor]]

Comment on attachment 8993063 [details]
Bug 1475722 - Mac Flash sandbox causes empty file upload dialogs on OS X 10.9, 10.10

https://reviewboard.mozilla.org/r/257874/#review265118

::: dom/plugins/base/nsPluginTags.cpp:465
(Diff revision 1)
> -      mSandboxLevel = ClampFlashSandboxLevel(mSandboxLevel);
> +      // Use standard level
> +      mSandboxLevel = Preferences::GetInt(levelPref, 0);
>      }
>  
> +    mSandboxLevel = ClampFlashSandboxLevel(mSandboxLevel);
> +    printf("Using Flash sandbox level %d\n", mSandboxLevel);

Remove this :-)

Comment 6

[Haik Aftandilian [:haik]]

Comment on attachment 8993063 [details]
Bug 1475722 - Mac Flash sandbox causes empty file upload dialogs on OS X 10.9, 10.10

https://reviewboard.mozilla.org/r/257874/#review265118

> Remove this :-)

Doh! Will do.

Comment 7

[Haik Aftandilian [:haik]]

Comment on attachment 8993063 [details]
Bug 1475722 - Mac Flash sandbox causes empty file upload dialogs on OS X 10.9, 10.10

https://reviewboard.mozilla.org/r/257874/#review265130

::: security/sandbox/mac/SandboxPolicies.h:700
(Diff revision 1)
> +  (if (or (string=? sandbox-level-2 "TRUE")
> +          (string=? sandbox-level-1 "TRUE") (begin

Was testing on 10.13 and found this "or" statement is wrong. The "begin" needs to be outside the "or" statement. Will fix and retest.

Comment 8

[Haik Aftandilian [:haik]]

Comment on attachment 8993063 [details]
Bug 1475722 - Mac Flash sandbox causes empty file upload dialogs on OS X 10.9, 10.10

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/257874/diff/1-2/

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/04c39e58ad7a
Mac Flash sandbox causes empty file upload dialogs on OS X 10.9, 10.10 r=Alex_Gaynor

Comment 10

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/04c39e58ad7a

Comment 11

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

I'd like us to verify the fixes in nightly for Bug 1475722, Bug 1471977, and Bug 1475707 before we uplift for beta. (Hopefully for beta 13 next Monday)

Comment 12

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

Comment on attachment 8993063 [details]
Bug 1475722 - Mac Flash sandbox causes empty file upload dialogs on OS X 10.9, 10.10

Approval Request Comment
[Feature/Bug causing the regression]: required for bug 1474375 
[User impact if declined]:
[Is this code covered by automated tests?]:
[Has the fix been verified in Nightly?]:
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]:
[Why is the change risky/not risky?]:
[String changes made/needed]:

Comment 13

[Oana Botisan, Desktop QA]

I managed to reproduce the issue using an older version of Nightly (2018-07-13) on Mac OS 10.10.
I retested everything using latest Nightly 63.0a1 on Mac OS 10.10 and Mac OS 10.9 and the bug is not reproducing anymore.

Comment 14

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

Thanks for the verify!

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8993063 [details]
Bug 1475722 - Mac Flash sandbox causes empty file upload dialogs on OS X 10.9, 10.10

Fix was verified on Nightly63, Beta62+

Comment 16

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3edfcf8b06b7

Comment 17

[Bogdan Maris, Desktop Test Engineering]

Adding qe-verify+ for beta verification as well.

Comment 18

[Ciprian Georgiu, Desktop QA]

Managed to reproduce this issue using an affected Nightly build from 2018-07-13, and following the STR from comment 0.

This bug is also verified fixed on Beta 62.0b14 (20180802174131), running Mac OS X 10.10 and 10.9.