Closed Bug 963974 Opened 10 years ago Closed 10 years ago

Faulty crash: use-after-free of mCurrentCompositeTask in CompositorParent::RecvFlushRendering()

Categories

(Core :: Graphics, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla30
Tracking Status
firefox27 --- wontfix
firefox28 + fixed
firefox29 + fixed
firefox30 + fixed
firefox-esr24 28+ fixed
b2g18 --- unaffected
b2g-v1.1hd --- unaffected
b2g-v1.2 --- fixed
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed

People

(Reporter: bjacob, Assigned: bjacob)

References

(Blocks 1 open bug)

Details

(Keywords: crash, csectype-uaf, sec-critical, Whiteboard: [qa-][adv-main28+][adv-esr24.4+])

Attachments

(3 files)

Found by Christoph Diehl's "Faulty" fuzzer, see bug 777067
Attached file Faulty session
I forgot to get a stack trace in that session @#$!
Classification: either PCompositor or non-protocol, IPC CancelableTask lifetime management bug (apparently IPC system destroying our CancelableTask under our feet), hard
Group: core-security
This is caused by forgetting to null the raw mCurrentCompositeTask after calling Cancel() on it.
Blocks: 873944
Attachment #8373653 - Flags: review?(matt.woodrow) → review+
Setting at least b2g 1.4 affected...
Assignee: nobody → bjacob
Comment on attachment 8373653 [details] [diff] [review]
Null mCurrentCompositeTask after calling Cancel() on it

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

An attacker might guess that if a secure patch is 1-line just clearing a pointer, then it's probable a use-after-free fix. Based on that, they could infer how to cause a use-after-free in the parent process. Then they would have to look at the specifics of CancelableTask to find an exploit (which is likely to be feasible, as this class is virtual). The patch doesn't give any hint about how to do that.

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 branches (regressed in gecko24).

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

1-line patch trivial to backport as needed.

How likely is this patch to cause regressions; how much testing does it need?

No risk at all.
Attachment #8373653 - Flags: sec-approval?
Comment on attachment 8373653 [details] [diff] [review]
Null mCurrentCompositeTask after calling Cancel() on it

sec-approval+ for trunk. I'd like to get Aurora, Beta, and ESR24 patches and nominations as well.
Attachment #8373653 - Flags: sec-approval? → sec-approval+
Does sec-approval+ mean green light to land on central?
Comment on attachment 8373653 [details] [diff] [review]
Null mCurrentCompositeTask after calling Cancel() on it

Not bothering to write "backported" versions of this 1-line patch, which is likely to apply just fine on various trees.


[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 873944
User impact if declined: use-after-free in compositor (parent) process
Testing completed (on m-c, etc.): no landing yet, since I'm waiting for an explicit green light to land on central. But locally this works fine.
Risk to taking this patch (and alternatives if risky): very low risk.
String or IDL/UUID changes made by this patch: none
Attachment #8373653 - Flags: approval-mozilla-esr24?
Attachment #8373653 - Flags: approval-mozilla-beta?
Attachment #8373653 - Flags: approval-mozilla-aurora?
(I've read the documentation now, so, yes, sec-approval+ did mean a green light to land :-) )
https://hg.mozilla.org/mozilla-central/rev/0d41267969e1
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Attachment #8373653 - Flags: approval-mozilla-esr24?
Attachment #8373653 - Flags: approval-mozilla-esr24+
Attachment #8373653 - Flags: approval-mozilla-beta?
Attachment #8373653 - Flags: approval-mozilla-beta+
Attachment #8373653 - Flags: approval-mozilla-aurora?
Attachment #8373653 - Flags: approval-mozilla-aurora+
Matt, can you please evaluate if this needs QA testing before we release Firefox 28?
Flags: needinfo?(mwobensmith)
QA Contact: mwobensmith
Flags: needinfo?(mwobensmith)
Whiteboard: [qa-]
Whiteboard: [qa-] → [qa-][adv-main28+][adv-esr24.4+]
Group: core-security
You need to log in before you can comment on or make changes to this bug.