Closed
Bug 1403707
Opened 7 years ago
Closed 7 years ago
Increase content sandbox job level to JOB_LOCKDOWN
Categories
(Core :: Security: Process Sandboxing, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox57 | --- | unaffected |
firefox58 | --- | fixed |
People
(Reporter: handyman, Assigned: handyman)
References
Details
(Whiteboard: sb+)
Attachments
(1 file, 2 obsolete files)
We are currently at content sandbox level 4, which has job level JOB_RESTRICTED. JOB_LOCKDOWN is JOB_RESTRICTED + JOB_OBJECT_LIMIT_DIE_ON_UNHANDLED_EXCEPTION [1]. This bug is just to strengthen the sandbox. The limit's docs are: "If an exception occurs and the system calls the UnhandledExceptionFilter function, the debugger will be given a chance to act. If there is no debugger, the functions returns EXCEPTION_EXECUTE_HANDLER. Normally, this will cause termination of the process with the exception code as the exit status." So this is about disabling the window that asks about Windows error reporting. I don't have an STR for this -- I don't know that I've seen the content process crash in a way that exposed the dialog. We use the flag that underlies this flag (SEM_NOGPFAULTERRORBOX) in SetErrorMode calls in two places -- both are predicated on XRE_NO_WINDOWS_CRASH_DIALOG being defined. I'm leaving the flag for (1) the other processes and (2) it also blocks the "find file..." dialog that comes up when the system looks for a file but can't find it (SEM_NOOPENFILEERRORBOX). GetErrorMode in content (and parent) is mostly just SEM_FAILCRITICALERRORS (although Win32 functions frequently, briefly, change it to finish their operations). This job flag doesn't seem to change that so I'm skeptical that it is working but it may just be implemented separately. [1] http://searchfox.org/mozilla-central/rev/3dbb47302e114219c53e99ebaf50c5cb727358ab/security/sandbox/chromium/sandbox/win/src/job.cc#39
Assignee | ||
Comment 1•7 years ago
|
||
Tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7d3ba16d364178046c053f5d0211687963a7a5e6
Assignee | ||
Updated•7 years ago
|
Attachment #8912892 -
Flags: review?(bobowencode)
Comment 2•7 years ago
|
||
Comment on attachment 8912892 [details] [diff] [review] Increase content sandbox job level to JOB_LOCKDOWN Review of attachment 8912892 [details] [diff] [review]: ----------------------------------------------------------------- Thanks. ::: security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp @@ +362,5 @@ > jobLevel = sandbox::JOB_LOCKDOWN; > accessTokenLevel = sandbox::USER_LOCKDOWN; > initialIntegrityLevel = sandbox::INTEGRITY_LEVEL_LOW; > delayedIntegrityLevel = sandbox::INTEGRITY_LEVEL_UNTRUSTED; > + } else if (aSandboxLevel >= 5) { Given that this hasn't shipped yet I think we should just add this to level 4.
Attachment #8912892 -
Flags: review?(bobowencode) → review+
Updated•7 years ago
|
Whiteboard: sb+
Assignee | ||
Comment 3•7 years ago
|
||
This one just changes level 4 to JOB_LOCKDOWN. Tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b85fd5a8a4dd909f175b864c421eccf85ea4f307
Attachment #8912892 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
status-firefox57:
--- → affected
Comment 4•7 years ago
|
||
Comment on attachment 8913318 [details] [diff] [review] Change content sandbox job level to JOB_LOCKDOWN (r=bobowen) Review of attachment 8913318 [details] [diff] [review]: ----------------------------------------------------------------- ::: security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp @@ +367,5 @@ > jobLevel = sandbox::JOB_RESTRICTED; > accessTokenLevel = sandbox::USER_LIMITED; > initialIntegrityLevel = sandbox::INTEGRITY_LEVEL_LOW; > delayedIntegrityLevel = sandbox::INTEGRITY_LEVEL_LOW; > + } else if (aSandboxLevel >= 4) { You still need to get rid of the >= 10 above or that would weaken the sandbox.
Assignee | ||
Comment 5•7 years ago
|
||
Boy, that was dumb. Here we go. Tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0390e01e071a792e38e3cdb6f5abfedb299a57a0
Attachment #8913318 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d018ea90b0c2 Change content sandbox job level to JOB_LOCKDOWN. r=bobowen
Keywords: checkin-needed
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d018ea90b0c2
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Comment 8•7 years ago
|
||
AFAIK, We are no longer at l4 on 57, so, updating the flags accordingly.
You need to log in
before you can comment on or make changes to this bug.
Description
•