Closed
Bug 1361733
Opened 7 years ago
Closed 7 years ago
[mac] Don't allow writes to all of /var/private in debug builds
Categories
(Core :: Security: Process Sandboxing, enhancement)
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: Alex_Gaynor, Assigned: Alex_Gaynor)
Details
(Whiteboard: sbmc2)
Attachments
(1 file)
https://bugzilla.mozilla.org/show_bug.cgi?id=1361304#c1 describes what we're trying to remove here, and what the blocker is.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Whiteboard: sbmc2
Comment 3•7 years ago
|
||
The particular error the /private/var DEBUG write-access rule was added for is the leakcheck test which is run for (all?) DEBUG tests. From https://bugzilla.mozilla.org/show_bug.cgi?id=1284588#c10 02:41:44 WARNING - TEST-UNEXPECTED-FAIL | leakcheck | tab process: missing output line for total leaks! 02:41:44 INFO - TEST-INFO | leakcheck | missing output line from log file /var/folders/f6/kjqp0l7n7cb307nv1hq0lrf400000w/T/tmpgFyenB.mozrunner/runtests_leaks_tab_pid1975.log
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → agaynor
Assignee | ||
Updated•7 years ago
|
Attachment #8864171 -
Flags: review?(haftandilian)
Assignee | ||
Comment 5•7 years ago
|
||
Try results here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ef3b88535c44ed701d8d8748c44345e1d417544&group_state=expanded
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8864171 [details] Bug 1361733 - In debug builds, do not allow content sandbox to write to all of /private/var https://reviewboard.mozilla.org/r/135834/#review139386 ::: commit-message-82c2d:1 (Diff revision 3) > +Bug 1361733 - In debug builds, do not allow content sandbox to write to all of /var/private Nit: /private/var instead of /var/private. ::: commit-message-82c2d:4 (Diff revision 3) > +Bug 1361733 - In debug builds, do not allow content sandbox to write to all of /var/private > + > +This permission was needed for the memory bloat logging -- specifically for > +logging intentionally crashing processes. Now we restrict ourselves to only And for the leaktest right? ::: dom/ipc/ContentChild.cpp:1286 (Diff revision 3) > > +// This function is only used in an |#ifdef DEBUG| path. > +#ifdef DEBUG > +// Given a path to a file, return the directory which contains it. > +static nsAutoCString > +GetDirectoryPath(char *path) { Can the path argument be const? Should be 'aPath' per coding guidelines. ::: layout/tools/reftest/runreftest.py:388 (Diff revision 3) > return None > browserEnv[v[:ix]] = v[ix + 1:] > > # Enable leaks detection to its own log file. > self.leakLogFile = os.path.join(profileDir, "runreftest_leaks.log") > - browserEnv["XPCOM_MEM_BLOAT_LOG"] = self.leakLogFile > + browserEnv["XPCOM_MEM_BLOAT_LOG"] = os.path.realpath(self.leakLogFile) Rather than depending on XPCOM_MEM_BLOAT_LOG not being a symlinked path, I think it would be better to make use of nsIFile::Normalize() in GetDirectoryPath(). With bug 1361304, I'm adding (allow file-read-metadata ..) rules for /private/var so the ::Normalize call should be OK.
Attachment #8864171 -
Flags: review?(haftandilian) → review-
Comment hidden (mozreview-request) |
Assignee | ||
Comment 8•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8864171 [details] Bug 1361733 - In debug builds, do not allow content sandbox to write to all of /private/var https://reviewboard.mozilla.org/r/135834/#review139386 > And for the leaktest right? Yes, indeed. Fixed.
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8864171 [details] Bug 1361733 - In debug builds, do not allow content sandbox to write to all of /private/var https://reviewboard.mozilla.org/r/135834/#review139684 I think this will have to land after 1361304 because it depends on resolving paths that link to within /private/var, but if tests are green no need to wait.
Attachment #8864171 -
Flags: review?(haftandilian) → review+
Comment hidden (mozreview-request) |
Comment 11•7 years ago
|
||
mozreview-review |
Comment on attachment 8864171 [details] Bug 1361733 - In debug builds, do not allow content sandbox to write to all of /private/var https://reviewboard.mozilla.org/r/135834/#review140168
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 12•7 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/45cc3ebc0446 In debug builds, do not allow content sandbox to write to all of /private/var r=haik
Keywords: checkin-needed
Comment 13•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/45cc3ebc0446
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•