Closed
Bug 1082986
Opened 10 years ago
Closed 10 years ago
Exploitable crash in mozilla::layers::ImageBridgeParent::SendFenceHandleIfPresent
Categories
(Core :: Graphics: Layers, defect)
Core
Graphics: Layers
Tracking
()
VERIFIED
FIXED
mozilla36
Tracking | Status | |
---|---|---|
firefox33 | --- | unaffected |
firefox34 | --- | unaffected |
firefox35 | + | verified |
firefox36 | + | fixed |
firefox-esr31 | --- | unaffected |
b2g-v1.4 | --- | unaffected |
b2g-v2.0 | --- | unaffected |
b2g-v2.0M | --- | unaffected |
b2g-v2.1 | --- | unaffected |
b2g-v2.2 | --- | fixed |
People
(Reporter: bc, Assigned: sotaro)
References
Details
(4 keywords)
Crash Data
Attachments
(5 files, 1 obsolete file)
126.47 KB,
text/plain
|
Details | |
180.49 KB,
text/plain
|
Details | |
184.49 KB,
text/plain
|
Details | |
21.74 KB,
patch
|
nical
:
review+
lsblakk
:
approval-mozilla-aurora+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
25.53 KB,
patch
|
sotaro
:
review+
|
Details | Diff | Splinter Review |
Bughunter has detected what appears to be an exploitable crash on Windows at least. It also crashes OSX 10.8 at least. The breakpad exploitable tool rates this as high. Microsoft's exploitable rates this as exploitable user mode dep. I have a semi saved version of the original NSFW page that reproduces but neither it nor the original url reproduce 100% of the time. I'll try to reduce. If not, I'll post the url and a zip of the saved version later. I'm filing now in case looking at the stacks helps identify the issue sooner. See also bug 1081706
Reporter | ||
Comment 1•10 years ago
|
||
As you can see, this one is much worse: Crash reason: EXCEPTION_ACCESS_VIOLATION_EXEC Crash address: 0xffffffffcccccccc
Reporter | ||
Comment 2•10 years ago
|
||
Updated•10 years ago
|
Component: IPC → Graphics: Layers
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → sotaro.ikeda.g
Assignee | ||
Comment 3•10 years ago
|
||
This code path is actually used only on b2g, the code could be disabled on platforms except b2g. But it does not fix the problem on b2g.
Assignee | ||
Comment 4•10 years ago
|
||
In current code, CompositableHost/TextureHost does not own Compositor's reference pointer. It could cause stale pointer's dereference. nial, do you know the reason of it?
Flags: needinfo?(nical.bugzilla)
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Sotaro Ikeda [:sotaro] from comment #4) > In current code, CompositableHost/TextureHost does not own Compositor's > reference pointer. It could cause stale pointer's dereference. It just owns Composotor's raw pointer.
Updated•10 years ago
|
Keywords: csectype-uaf,
sec-critical
Comment 7•10 years ago
|
||
Sotaro, what bug is this a regression from?
Assignee | ||
Comment 8•10 years ago
|
||
Bug 1067455 is a direct cause of the regression. Potential problem was added since Bug 825928 fix.
Comment 9•10 years ago
|
||
Thanks! [Tracking Requested - why for this release]: sec-high.
Blocks: 1067455
status-firefox34:
--- → unaffected
status-firefox35:
--- → affected
status-firefox36:
--- → affected
tracking-firefox35:
--- → ?
tracking-firefox36:
--- → ?
Keywords: regressionwindow-wanted
Assignee | ||
Comment 10•10 years ago
|
||
Assignee | ||
Comment 11•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=6e411d966843
Assignee | ||
Updated•10 years ago
|
Attachment #8505692 -
Flags: review?(nical.bugzilla)
Comment 12•10 years ago
|
||
Comment on attachment 8505692 [details] [diff] [review] patch - Use RefPtr to hold Compositor Review of attachment 8505692 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/opengl/CompositingRenderTargetOGL.h @@ +156,5 @@ > */ > void InitializeImpl(); > > InitParams mInitParams; > + RefPtr<CompositorOGL> mCompositor; nit: There is temporary a cycle between the compositor and the render target, each having a strong ref to the other. It's okay since the compositor's reference to the target is always cleared at the end of a frame, but it would be nice to add a comment, maybe in Compositor.h around SetRenderTarget to say that the reference must only be kept during the duration of the frame. Otherwise I am worried that we will eventually break this equilibrium by keeping the target after the frames or run into this every time we run some cycle detection tools like bjacob did a little while ago.
Attachment #8505692 -
Flags: review?(nical.bugzilla) → review+
Assignee | ||
Comment 13•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c90324964f7
Assignee | ||
Comment 14•10 years ago
|
||
(In reply to Sotaro Ikeda [:sotaro] from comment #13) > https://hg.mozilla.org/integration/mozilla-inbound/rev/3c90324964f7 I hope it fix the crash.
Reporter | ||
Comment 15•10 years ago
|
||
The crash was not very reproducible. I could not reliably detect it with the saved version of the page and the live version was only reproducible maybe once in a hundred attempts. Once this lands on mozilla-central, I will run a retest over all the workers and will do repeated attempts on windows 7 for at least 200 attempts. I'll let you know the results when it completes.
Assignee | ||
Comment 16•10 years ago
|
||
Thanks!
Comment 17•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3c90324964f7 This should have had sec-approval before landing, BTW.
Status: NEW → RESOLVED
Closed: 10 years ago
status-b2g-v1.4:
--- → unaffected
status-b2g-v2.0:
--- → unaffected
status-b2g-v2.0M:
--- → unaffected
status-b2g-v2.1:
--- → unaffected
status-b2g-v2.2:
--- → fixed
status-firefox33:
--- → unaffected
status-firefox-esr31:
--- → unaffected
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Comment 18•10 years ago
|
||
We'll need to get the questions answered from the sec-approval? template before we can determine whether to take this on Aurora.
Reporter | ||
Comment 19•10 years ago
|
||
I could not reproduce this crash on Nightly on Win7 32bit after this fix landed.
Assignee | ||
Comment 20•10 years ago
|
||
Cool! Thanks :-)
Assignee | ||
Comment 21•10 years ago
|
||
Comment on attachment 8505692 [details] [diff] [review] patch - Use RefPtr to hold Compositor [Security approval request comment] How easily could an exploit be constructed based on the patch? - It seems difficult to create the exploit based on this patch. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? - No tests included. One comment is added. It is not related to the security. Which older supported branches are affected by this flaw? - firefox35 is affected. If not all supported branches, which bug introduced the flaw? - Bug 1067455 is a direct cause of the regression. - Potential problem was added since Bug 825928 fix. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? - The same patch seems to work for firefox35. create a patch for firefox35 is easy and low risk. How likely is this patch to cause regressions; how much testing does it need? - This fix is low risk of regression. I tested locally and tryserver test passed. And comment 19 did confirmation.
Attachment #8505692 -
Flags: sec-approval?
Comment 22•10 years ago
|
||
Comment on attachment 8505692 [details] [diff] [review] patch - Use RefPtr to hold Compositor Sounds good. Can we get an Aurora patch (or a nomination on the existing patch if it applies) so we can check this in there as well?
Attachment #8505692 -
Flags: sec-approval? → sec-approval+
Assignee | ||
Comment 23•10 years ago
|
||
A patch for aurora is necessary. I am preparing it.
Assignee | ||
Comment 24•10 years ago
|
||
Attachment #8507265 -
Flags: review+
Assignee | ||
Comment 25•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=777fb68d8cf2
Assignee | ||
Comment 26•10 years ago
|
||
Comment on attachment 8505692 [details] [diff] [review] patch - Use RefPtr to hold Compositor Approval Request Comment [Feature/regressing bug #]:Bug 1067455 [User impact if declined]:security risk. And chrome process crash. [Describe test coverage new/current, TBPL]:locally tested. [Risks and why]: low [String/UUID change made/needed]: none
Attachment #8505692 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
Updated•10 years ago
|
Attachment #8505692 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 28•10 years ago
|
||
Backed out for bustage. https://hg.mozilla.org/releases/mozilla-aurora/rev/16313092de82 https://treeherder.mozilla.org/ui/logviewer.html#?job_id=337275&repo=mozilla-aurora
Assignee | ||
Comment 29•10 years ago
|
||
Sorry, patch for aurora attachment 8507265 [details] [diff] [review] was different from Comment 25. I uploaded just the master's patch.
Assignee | ||
Comment 30•10 years ago
|
||
Correct patch for aurora. Carry "r=nical".
Attachment #8507265 -
Attachment is obsolete: true
Attachment #8508978 -
Flags: review+
Assignee | ||
Comment 31•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=6c59005e1996
Comment 32•10 years ago
|
||
Whoops :) https://hg.mozilla.org/releases/mozilla-aurora/rev/e49b50024257
Comment 33•9 years ago
|
||
Hi Bob, can you verify that this has been fixed in Fx35? Thank you.
Flags: needinfo?(bob)
Reporter | ||
Comment 34•9 years ago
|
||
Matt, will do. I'm doing a quick test on Windows and OSX with one attempt per beta, aurora, nightly and a longer attempt on winxp, win7 32/64 on beta only where I'm loading the url 200 times each. I'll let you know later this evening how it goes.
Reporter | ||
Updated•9 years ago
|
Flags: needinfo?(bob)
Reporter | ||
Comment 36•9 years ago
|
||
I could not reproduce the crash with a Nightly built from today.
Updated•9 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•