This contains: AutoRestore<bool> autoRestoreRunningSample(mRunningSample); ... nsCOMPtr<nsIDocument> document(mDocument); // keeps 'this' alive too If |document| keeps this alive, then it is possible that destroying |document| will destroy |this|, which means the restore will be a use after free.
Similarly in nsSMILInstanceTime::HandleChangedInterval.
Summary: Possible UAF in nsSMILAnimationController::DoSample → Possible UAFs with AutoRestore in SMIL code
The only code we're running in between the two is destroying two nsTHashtables, so it would be hard to exploit it. (Well, aside from whatever is going on with other threads.)
Assignee: nobody → bbirtles
Status: NEW → ASSIGNED
Attachment #8837443 - Flags: review?(continuation)
Attachment #8837443 - Flags: review?(continuation) → review+
sec-moderate so landing this myself: https://hg.mozilla.org/integration/mozilla-inbound/rev/33f49f751b63cde40ffe16d4c5fee0b4a64a6a79
Please request Aurora/Beta approval on this when you get a chance.
Comment on attachment 8837443 [details] [diff] [review] Patch Approval Request Comment [Feature/Bug causing the regression]: bug 814921 [User impact if declined]: Unpatched user-after-free (although it appears hard to exploit) [Is this code covered by automated tests?]: The SMIL functionality is well-tested but there is not specific test case for this vulnerability. [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No [Why is the change risky/not risky?]: It just re-orders two stack based classes so that their destructors are run in a safe order. [String changes made/needed]: None
Comment on attachment 8837443 [details] [diff] [review] Patch Fix a security issue. Aurora53+.
Attachment #8837443 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8837443 [details] [diff] [review] Patch fix UAF in SMIL, beta52+
Attachment #8837443 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main52+]
Component: Layout: Misc Code → Layout
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.