Content processes shouldn't be able to read $PROFILE/weave sync data

RESOLVED FIXED in Firefox 55

Status

()

Core
Security: Process Sandboxing
RESOLVED FIXED
a year ago
8 months ago

People

(Reporter: haik, Assigned: Alex_Gaynor)

Tracking

51 Branch
mozilla55
Unspecified
All
Points:
---

Firefox Tracking Flags

(firefox51 affected, firefox55 fixed)

Details

(Whiteboard: sblc3,sbwc3,sbmc2)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

a year ago
On OS X, the content process sandbox permits read access to the weave subdirectory within the user's profile. For example,

  /Users/haik/Library/Application Support/Firefox/Profiles/hcn9pyni.synctest/weave/

There is sensitive data here (passwords, bookmarks, etc.) and content processes shouldn't be allowed to read those files.

The rule allowing content to access this directory was added for bug 1175881 "about:sync-log can't read files on OS X with e10s on and content process sandbox enabled".

But about:sync-log only appears to need access to the logs subdirectory within the weave directory. From what I've found, the content process only needs access to $PROFILE/weave for about:sync-log, not for correct sync operation.

Solution 1) I think the best solution here would be to make about:sync-log open in the chrome process like about:crashes. Note: about:sync-log opens file:///Users/.../$PROFILE/weave/logs/ and includes the "Up to higher level directory" navigation link. If about:sync-log is opened in the parent, I think we'd want that link to be removed.

Solution 2) Once we have multi-content processes and a file-system specific content process, about:sync-logs could be opened in the file-system specific content process because that process will presumably have read access to the user's entire home directory.

Solution 3) For now, we could change the content sandbox rule to only allow reading of the $PROFILE/weave/logs directory, not the entire $PROFILE/weave. The logs files entries appear to have sensitive information scrubbed from them, but that's TBD.
(Reporter)

Updated

a year ago
Summary: OS X Content processes shouldn't be able to read the $PROFILE/weave sync data → OS X Content processes shouldn't be able to read $PROFILE/weave sync data
Whiteboard: sb?
(Reporter)

Comment 1

a year ago
(In reply to Haik Aftandilian [:haik] from comment #0)
> Solution 1) I think the best solution here would be to make about:sync-log
> open in the chrome process like about:crashes. Note: about:sync-log opens
> file:///Users/.../$PROFILE/weave/logs/ and includes the "Up to higher level
> directory" navigation link. If about:sync-log is opened in the parent, I
> think we'd want that link to be removed.

I was wrong about this. about:sync-log is opened in chrome and the directory listing is generated there, but click on a log entry results in the log file being loaded in content.

> Solution 2) Once we have multi-content processes and a file-system specific
> content process, about:sync-logs could be opened in the file-system specific
> content process because that process will presumably have read access to the
> user's entire home directory.

When we have a dedicated file-system content process, this should work without the content process needing access to $PROFILE/weave.

> Solution 3) For now, we could change the content sandbox rule to only allow
> reading of the $PROFILE/weave/logs directory, not the entire $PROFILE/weave.
> The logs files entries appear to have sensitive information scrubbed from
> them, but that's TBD.

I think it would make sense to change the current content rules to only allow content to access $PROFILE/weave/logs letting about:sync-log still work but removing content process access to more sensitive parts of the weave directory.
(Reporter)

Updated

a year ago
See Also: → bug 1147911

Updated

a year ago
Whiteboard: sb? → sblc3,sbwc2,sbmc2
(Reporter)

Updated

a year ago
Depends on: 1147911

Updated

a year ago
OS: Mac OS X → All
Summary: OS X Content processes shouldn't be able to read $PROFILE/weave sync data → Content processes shouldn't be able to read $PROFILE/weave sync data

Updated

11 months ago
Whiteboard: sblc3,sbwc2,sbmc2 → sblc3,sbwc3,sbmc2
(Assignee)

Updated

8 months ago
Assignee: nobody → agaynor
(Assignee)

Comment 2

8 months ago
We now have a file content process, yay! Since about:sync-log loads in the chrome process (bug 1354241 tracks moving it to a file content process of its own), and any browsing you do to a log file gets directed to a file content process, all that's left to do here is remove access to this directory.

macOS: Remove from Sandbox.mm's policy
Windows: I need to learn how this works
Linux: Blocked on bug 1308400
Comment hidden (mozreview-request)
(Assignee)

Updated

8 months ago
Attachment #8855445 - Flags: review?(haftandilian)
(Assignee)

Comment 4

8 months ago
Ok, if I've read the Windows code correctly, I don't _think_ it's affected by this.

A small note on macOS: with this patch if you disable |browser.tabs.remote.separateFileUriProcess| then clicking links from about:sync-log stops working, because the content process no longer has access (yay!). This is a general extension of the fact that without that pref, browsing in the profile dir is generally broken.

Currently this pref is only enabled on nightly, it's disabled on other channels.
(Reporter)

Comment 5

8 months ago
mozreview-review
Comment on attachment 8855445 [details]
Bug 1295700 - Don't allow content processes to access the weave director on macOS

https://reviewboard.mozilla.org/r/127304/#review130032
Attachment #8855445 - Flags: review?(haftandilian) → review+
Comment hidden (mozreview-request)
(Assignee)

Updated

8 months ago
Keywords: checkin-needed

Comment 7

8 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/afccb72dba23
Don't allow content processes to access the weave director on macOS r=haik
Keywords: checkin-needed

Comment 8

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/afccb72dba23
Status: NEW → RESOLVED
Last Resolved: 8 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55

Comment 9

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/afccb72dba23
You need to log in before you can comment on or make changes to this bug.