Closed Bug 1264240 Opened 8 years ago Closed 8 years ago

Allow NPAPI sandbox write access to %LOCALAPPDATA%\Macromedia\Flash Player.

Categories

(Core :: Security: Process Sandboxing, defect)

x86_64
Windows
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox47 + fixed
firefox48 --- fixed

People

(Reporter: bobowen, Assigned: bobowen)

Details

(Whiteboard: sbwn1)

Attachments

(1 file)

Adobe have requested that the Windows 64-bit NPAPI sandbox allows write access to %LOCALAPPDATA%\Macromedia\Flash Player.
Hi KT, hopefully this build from the try push in comment 1 will give the access you require.

https://archive.mozilla.org/pub/firefox/try-builds/bobowencode@gmail.com-7062bba16758cc856b123c0d492564652faedb6f/try-win64/firefox-48.0a1.en-US.win64.installer.exe

I've only given access to the "Flash Player" directory under %LOCALAPPDATA%\Macromedia\ to match the existing access.
It should also give access to create the directory structure as well.

Let me know and I'll get this reviewed and landed.
Flags: needinfo?(ktakkim)
Thanks Bob!!
"comment 1" looks good to me.
Flags: needinfo?(ktakkim)
MozReview-Commit-ID: C6PMpIcFMUb
Attachment #8741044 - Flags: review?(jmathies)
Comment on attachment 8741044 [details] [diff] [review]
Allow NPAPI sandbox write access to %LOCALAPPDATA%\Macromedia\Flash Player

Review of attachment 8741044 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/plugins/ipc/PluginProcessParent.cpp
@@ +111,5 @@
>                            NS_LITERAL_STRING("\\Macromedia"));
> +    AddSandboxAllowedFile(aAllowedDirectories, dirSvc, NS_WIN_LOCAL_APPDATA_DIR,
> +                          NS_LITERAL_STRING("\\Macromedia\\Flash Player"));
> +    AddSandboxAllowedFile(aAllowedDirectories, dirSvc, NS_WIN_LOCAL_APPDATA_DIR,
> +                          NS_LITERAL_STRING("\\Macromedia"));

nit, can we swap these and other dir rules such that "\\ABC" lists before "\\ABC\\Flash Player"?
Attachment #8741044 - Flags: review?(jmathies) → review+
Whiteboard: sb? → sbwn1
(In reply to Jim Mathies [:jimm] from comment #5)

> > +    AddSandboxAllowedFile(aAllowedDirectories, dirSvc, NS_WIN_LOCAL_APPDATA_DIR,
> > +                          NS_LITERAL_STRING("\\Macromedia\\Flash Player"));
> > +    AddSandboxAllowedFile(aAllowedDirectories, dirSvc, NS_WIN_LOCAL_APPDATA_DIR,
> > +                          NS_LITERAL_STRING("\\Macromedia"));
> 
> nit, can we swap these and other dir rules such that "\\ABC" lists before
> "\\ABC\\Flash Player"?

Thanks Jim.
Yes, I think it makes sense to have the file ones first as they'll be used much more.
If I remember correctly it searches through in order for a match.
The directory order doesn't matter much, so I'll do as you suggest to make it more readable.
https://hg.mozilla.org/mozilla-central/rev/1942e832fd7a
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
[Tracking Requested - why for this release]:
committed to uplifting this for adobe
Comment on attachment 8741044 [details] [diff] [review]
Allow NPAPI sandbox write access to %LOCALAPPDATA%\Macromedia\Flash Player

Note to sheriff: patch that landed had some lines moved around to aid readability.

Approval Request Comment
[Feature/regressing bug #]:
New feature requested by Adobe, required for June release.

[User impact if declined]:
Flash Player feature will not operate correctly for 64-bit Firefox.

[Describe test coverage new/current, TreeHerder]:
Automated NPAPI tests in tree.

[Risks and why]:
Low: Very simple addition of new sandbox policy rules.

[String/UUID change made/needed]:
None
Attachment #8741044 - Flags: approval-mozilla-aurora?
Comment on attachment 8741044 [details] [diff] [review]
Allow NPAPI sandbox write access to %LOCALAPPDATA%\Macromedia\Flash Player

Partner ask, Aurora47+
Attachment #8741044 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.