Closed
Bug 1318012
Opened 8 years ago
Closed 7 years ago
SandboxBrokerCommon::SendWithFd sends uninitialised stack-allocated data out of process
Categories
(Core :: Security: Process Sandboxing, defect)
Core
Security: Process Sandboxing
Tracking
()
RESOLVED
FIXED
mozilla53
People
(Reporter: jseward, Assigned: jseward)
Details
(Keywords: csectype-uninitialized, sec-moderate, Whiteboard: sblc2, [post-critsmash-triage])
Attachments
(2 files, 1 obsolete file)
3.46 KB,
text/plain
|
Details | |
1.64 KB,
patch
|
jld
:
review+
|
Details | Diff | Splinter Review |
I ran gtests on valgrind and noticed that SandboxBrokerCommon::SendWithFd can send uninitialised stack allocated data out of process. I suspect this is due to alignment holes in char cmsg_buf[] in that function. Partly because it seems ungood to have an information leak, and partly to stop valgrind complaining, I'd prefer to zero initialise that array. It's only 24 bytes, at least on x86_64-linux.
Assignee | ||
Comment 1•8 years ago
|
||
A possible fix.
Assignee | ||
Comment 2•8 years ago
|
||
Running gtests on valgrind is a bit tricky. STR: run valgrind-listener in a different shell. This listens on port 1500. export MOZVFLAGS="--num-transtab-sectors=24 --px-default=allregs-at-mem-access \ --px-file-backed=unwindregs-at-mem-access --fair-sched=yes \ --fullpath-after=/MOZ/ --trace-children=yes --show-mismatched-frees=no" DISPLAY=:1.0 G_SLICE=always-malloc v312BRANCH $MOZVFLAGS \ "--trace-children-skip=/usr/bin/which,/usr/bin/file,/usr/bin/uname,/sbin/ldconfig,/usr/bin/cat,/usr/bin/awk,/usr/bin/sed,/usr/bin/whoami,/usr/bin/true,/*/bin/gcc,/usr/bin/gmake" \ --track-origins=yes --log-socket=127.0.0.1:1500 --num-callers=40 \ ./mach gtest
Assignee | ||
Comment 3•8 years ago
|
||
Attachment #8811327 -
Attachment is obsolete: true
Attachment #8811617 -
Flags: review?(jld)
Updated•8 years ago
|
Whiteboard: sblc2
Updated•8 years ago
|
Keywords: csectype-uninitialized,
sec-moderate
Updated•8 years ago
|
Assignee: nobody → jseward
Updated•8 years ago
|
Attachment #8811617 -
Flags: review?(jld) → review+
Comment 4•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/55d9f54c1a58
Target Milestone: --- → mozilla53
Updated•8 years ago
|
Group: core-security → dom-core-security
Updated•7 years ago
|
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Comment 5•7 years ago
|
||
The sec-* keywords on this bug might be a bit on the high side — at worst it's an information leak of a few bytes (and I haven't verified that the bytes actually are copied to the other process), it always reads from the same place on the broker thread's stack, _and_ it needs to be combined with a sec-critical to do anything.
Updated•7 years ago
|
Group: dom-core-security → core-security-release
Comment 6•7 years ago
|
||
Should we consider backporting this to 52 for the next ESR?
status-firefox51:
--- → wontfix
status-firefox52:
--- → affected
Updated•7 years ago
|
Flags: needinfo?(jseward)
Comment 7•7 years ago
|
||
Thanks for checking, but there's no benefit to uplifting this. The file broker is used only for content processes, and content sandboxing won't be enabled for non-nightly builds until 53 at the earliest. (I'm not sure if "unaffected" is the right flag, because nightly 52 *was* affected, but aurora/beta/release/etc. 52 aren't.)
Flags: needinfo?(jseward)
Comment 8•7 years ago
|
||
We generally use the disabled status for features that aren't riding that train. Thanks!
Updated•7 years ago
|
Flags: qe-verify-
Whiteboard: sblc2 → sblc2, [post-critsmash-triage]
Updated•7 years ago
|
Group: core-security-release
status-firefox-esr52:
--- → disabled
You need to log in
before you can comment on or make changes to this bug.
Description
•