Closed Bug 790856 Opened 8 years ago Closed 7 years ago

Window resize accessed a dangling DocumentViewerImpl

Categories

(Core :: DOM: Navigation, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla18
Tracking Status
firefox15 --- wontfix
firefox16 + fixed
firefox17 + fixed
firefox18 + fixed
firefox-esr10 16+ fixed

People

(Reporter: jruderman, Assigned: smaug)

References

(Blocks 1 open bug)

Details

(Keywords: csectype-uaf, sec-critical, Whiteboard: [asan][advisory-tracking+][qa-])

Attachments

(2 files)

Attached file stack traces from ASan
I wasn't able to reproduce this.  Can you figure it out from the stacks, and knowing that the fuzzer was opening and closing windows?
Probably.
  if (mPreviousViewer)
    mPreviousViewer->SetBounds(aBounds);
looks suspicious. We call set bounds on the previous viewer, and at least in theory that could
end up doing stuff which sets mPreviousViewer null.
Attached patch patchSplinter Review
Jesse, want to try this? Though, if reproducing is hard, verifying the
fix can be difficult.
Attachment #660748 - Flags: review?(bzbarsky)
Do we know what versions are affected by this bug?
Assignee: nobody → bugs
If my patch fixes the problem, then everything after 2.0 or something.
Comment on attachment 660748 [details] [diff] [review]
patch

r=me
Attachment #660748 - Flags: review?(bzbarsky) → review+
Comment on attachment 660748 [details] [diff] [review]
patch

[Approval Request Comment]
User impact if declined: possible crash
Fix Landed on Version: NA
Risk to taking this patch (and alternatives if risky): Should be super-safe
String or UUID changes made by this patch: NA

[Approval Request Comment]
Regression caused by (bug #): Bug 290991
User impact if declined: Possible crashes
Testing completed (on m-c, etc.): NA
Attachment #660748 - Flags: approval-mozilla-release?
Attachment #660748 - Flags: approval-mozilla-esr10?
Attachment #660748 - Flags: approval-mozilla-beta?
Attachment #660748 - Flags: approval-mozilla-aurora?
Comment on attachment 660748 [details] [diff] [review]
patch

[Triage Comment]
Approving for all unreleased branches given this is sec-critical.
Attachment #660748 - Flags: approval-mozilla-release?
Attachment #660748 - Flags: approval-mozilla-release-
Attachment #660748 - Flags: approval-mozilla-esr10?
Attachment #660748 - Flags: approval-mozilla-esr10+
Attachment #660748 - Flags: approval-mozilla-beta?
Attachment #660748 - Flags: approval-mozilla-beta+
Attachment #660748 - Flags: approval-mozilla-aurora?
Attachment #660748 - Flags: approval-mozilla-aurora+
Oh, did I accidentally ask a? for release. Wasn't going to.
Duplicate of this bug: 791534
(In reply to Olli Pettay [:smaug] from comment #3)
> Created attachment 660748 [details] [diff] [review]
> patch
> 
> Jesse, want to try this? Though, if reproducing is hard, verifying the
> fix can be difficult.

Tested your patch, it indeed fixes the crash.
Keywords: csec-uaf
(In reply to Olli Pettay [:smaug] from comment #9)
> Oh, did I accidentally ask a? for release. Wasn't going to.

Given the verification, please land asap on all branches. Thanks!
Whiteboard: [asan] → [asan][advisory-tracking+]
Whiteboard: [asan][advisory-tracking+] → [asan][advisory-tracking+][qa-]
Group: core-security
You need to log in before you can comment on or make changes to this bug.