Closed Bug 1307282 Opened 3 years ago Closed 3 years ago

Remove global file-read-metadata rule and unused macros from OS X content sandbox

Categories

(Core :: Security: Process Sandboxing, defect)

51 Branch
Unspecified
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: haik, Assigned: haik)

Details

(Whiteboard: sbmc2)

Attachments

(2 files)

This bug addresses a couple of low priority policy cleanup opportunities.

1) The content sandbox has the redundant rule (allow file-read-metadata) which allows reading of metadata for any file the user has access to. It's redundant because other rules, specifically the level 1 and 2 specific rules, setup the global filesystem access depending of the level. For example, for level 1 we allow read access to anywhere on the filesystem that the OS permits with 

  ; level 1: global read access permitted, no home write access
    (if (= sandbox-level 1)\n"
      (begin\n"
        (allow file-read*)\n"
        (allow file-write* (require-not (subpath home-path)))))

so having another rule allowing global file-read-metadata is error prone and confusing.

2) We can also remove these unused macros.

  (define (container-regex container-relative-regex) ...)
  (define (container-subpath container-relative-subpath) ...)
  (define (container-literal container-relative-literal) ...)
  (define (appdir-regex appdir-relative-regex) ...)
  (define (appdir-subpath appdir-relative-subpath) ...)
  (define (appdir-literal appdir-relative-literal) ...)
Assignee: nobody → haftandilian
Whiteboard: sbmc2
Try results with just the (allow file-read-metadata) removed.

  https://treeherder.mozilla.org/#/jobs?repo=try&revision=3ec42b81f875
Comment on attachment 8797315 [details]
Bug 1307282 - Remove unused sandbox ruleset macros;

https://reviewboard.mozilla.org/r/82900/#review82892
Attachment #8797315 - Flags: review?(gpascutto) → review+
Comment on attachment 8797316 [details]
Bug 1307282 - Remove redundant read-metadata rights from the content sandbox;

https://reviewboard.mozilla.org/r/82902/#review82894
Attachment #8797316 - Flags: review?(gpascutto) → review+
Keywords: checkin-needed
Why do these two commits have identical commit messages?
Flags: needinfo?(haftandilian)
Keywords: checkin-needed
(In reply to Ryan VanderMeulen [:RyanVM] from comment #6)
> Why do these two commits have identical commit messages?

Fixed. Sorry, didn't realize that was a problem. Both commit messages had the same first line and differentiating second lines.
Flags: needinfo?(haftandilian) → needinfo?(ryanvm)
Keywords: checkin-needed
Flags: needinfo?(ryanvm)
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/0a7e549e1e91
Remove unused sandbox ruleset macros; r=gcp
https://hg.mozilla.org/integration/autoland/rev/e80c8083d933
Remove redundant read-metadata rights from the content sandbox; r=gcp
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/0a7e549e1e91
https://hg.mozilla.org/mozilla-central/rev/e80c8083d933
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.