Closed
Bug 969549
Opened 11 years ago
Closed 11 years ago
Faulty: PCompositableTransaction reinterprets between layer types based on untrusted message params
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: bjacob, Assigned: bjacob)
References
(Blocks 1 open bug)
Details
(Keywords: sec-critical, Whiteboard: [adv-main30+][qa-])
Attachments
(2 files)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
nical
:
review+
dveditz
:
sec-approval+
|
Details | Diff | Splinter Review |
Found by Christoph Diehl's "Faulty" fuzzer, see bug 777067
This is similar to bug 968833, but now for the PCompositableTransaction protocol.
Assignee | ||
Comment 1•11 years ago
|
||
Similar to bug 968833, I suggest sec-crit here. While the present faulty session gives a nice assert failure, that's because it was only calling a virtual method that turned out to be asserting on the current object type. Looking at CompositableTransactionParent.cpp there are places that static_cast between layer types based on untrusted inputs exactly like bug 968833.
Assignee | ||
Comment 2•11 years ago
|
||
To give just one example:
http://hg.mozilla.org/mozilla-central/file/a96affc08b1c/gfx/layers/ipc/CompositableTransactionParent.cpp#l158
case CompositableOperation::TOpPaintTextureRegion: {
MOZ_LAYERS_LOG(("[ParentSide] Paint ThebesLayer"));
const OpPaintTextureRegion& op = aEdit.get_OpPaintTextureRegion();
CompositableParent* compositableParent = static_cast<CompositableParent*>(op.compositableParent());
CompositableHost* compositable =
compositableParent->GetCompositableHost();
ThebesLayerComposite* thebes =
static_cast<ThebesLayerComposite*>(compositable->GetLayer());
Assignee | ||
Comment 3•11 years ago
|
||
Interesting bit in this Faulty log:
[Faulty] pickle field {int} of value: 8 changed to: 7
In CompositableType enum, 8 is COMPOSITABLE_IMAGE, 7 is BUFFER_TILED.
Updated•11 years ago
|
Keywords: sec-critical
Updated•11 years ago
|
status-firefox30:
--- → affected
tracking-firefox30:
--- → +
Assignee | ||
Comment 4•11 years ago
|
||
hit an avatar of this in bug 971262.
Patch coming up.
Assignee | ||
Comment 5•11 years ago
|
||
Attachment #8374559 -
Flags: review?(nical.bugzilla)
Updated•11 years ago
|
Attachment #8374559 -
Flags: review?(nical.bugzilla) → review+
Updated•11 years ago
|
status-b2g-v1.4:
--- → affected
Assignee | ||
Comment 7•11 years ago
|
||
Comment on attachment 8374559 [details] [diff] [review]
fix-PCompositableTransaction-casts
[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Not easily. To someone with good understanding of our codebase (please send your resumes) the patch gives away that passing forged messages to PCompositorTransaction can result in unsafe memory accesses. It remains to figure how exactly to forge such messages (not hard, since it was found by fuzzing generic pickles) and how to actually exploit that (harder).
Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
No.
Which older supported branches are affected by this flaw?
All of them.
Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
No backport ready, but wouldn't be difficult.
How likely is this patch to cause regressions; how much testing does it need?
Not very risky, but not trivial.
Attachment #8374559 -
Flags: sec-approval?
Updated•11 years ago
|
Comment 8•11 years ago
|
||
Comment on attachment 8374559 [details] [diff] [review]
fix-PCompositableTransaction-casts
sec-approval=dveditz
Attachment #8374559 -
Flags: sec-approval? → sec-approval+
Assignee | ||
Comment 9•11 years ago
|
||
Updated•11 years ago
|
status-firefox29:
--- → wontfix
Comment 10•11 years ago
|
||
landed on central https://hg.mozilla.org/mozilla-central/rev/3bfe9190b72d
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Updated•11 years ago
|
status-firefox28:
--- → wontfix
status-firefox-esr24:
--- → unaffected
Comment 11•11 years ago
|
||
v1.1 will be EOL on Monday, so it seems highly unlikely that this is going to be uplifted there :)
status-b2g-v1.1hd:
--- → wontfix
Comment 12•11 years ago
|
||
If we were going to backport these to 1.2 and 1.3, that time was a long time ago.
Updated•10 years ago
|
Whiteboard: [adv-main30+]
Comment 13•10 years ago
|
||
Marking [qa-] for desktop QA verification. FxOS QA may choose to verify at a later date.
Whiteboard: [adv-main30+] → [adv-main30+][qa-]
Comment 14•10 years ago
|
||
Applied cleanly to SeaMonkey 2.26.1 (Gecko 29-based)
https://hg.mozilla.org/releases/mozilla-release/rev/304350ccb7ac
status-seamonkey2.26:
--- → fixed
Updated•9 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•