Crash when window gets destroyed during popuphiding event

RESOLVED FIXED

Status

()

Core
XUL
--
critical
RESOLVED FIXED
11 years ago
a year ago

People

(Reporter: Martijn Wargers (dead), Assigned: smaug)

Tracking

(4 keywords)

Trunk
x86
Windows XP
crash, testcase, verified1.8.0.12, verified1.8.1.4
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.1.4 +
blocking1.8.0.12 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical])

Attachments

(3 attachments)

(Reporter)

Description

11 years ago
This is similar to bug 336739, but now with the popuphiding event (that's why I also made this bug security sensitive).

This also crashes Mozilla1.7.

Talkback ID: TB18335801Z
nsPopupSetFrame::OpenPopup   nsPopupSetFrame::DestroyPopup   nsPopupBoxObject::HidePopup   XPCWrappedNative::CallMethod
(Reporter)

Comment 1

11 years ago
Created attachment 220933 [details]
testcase (crashes within 1 sec)
taking for now
Assignee: jag → Olli.Pettay
Created attachment 256914 [details] [diff] [review]
proposed patch

Better to use only objects which are guaranteed to be alive.
And PresShell should not die while dispatching an event.
Attachment #256914 - Flags: superreview?(roc)
Attachment #256914 - Flags: review?(roc)
Status: NEW → ASSIGNED
Attachment #256914 - Flags: superreview?(roc)
Attachment #256914 - Flags: superreview+
Attachment #256914 - Flags: review?(roc)
Attachment #256914 - Flags: review+
checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Attachment #256914 - Flags: approval1.8.1.3?
On a trunk debug build (before fix) this crashes trying to release an already-deleted object (after we've already been doing stuff with that object for a while). Definitely a security risk involved.

QA note: The testcase on this bug doesn't crash me on an optimized 1.8.0.10 or 1.8.1.3, but does in a debug build (slightly different place, but still a deleted object). The branch fix will have to be verified in a debug build

Flags: blocking1.8.1.4+
Flags: blocking1.8.0.12+
Whiteboard: [sg:critical]
Attachment #256914 - Flags: approval1.8.1.4?
Created attachment 259120 [details] [diff] [review]
for branches

just the change that mPresContext is used in branches, trunk has
GetPresContext()
Attachment #259120 - Flags: approval1.8.1.4?
Attachment #259120 - Flags: approval1.8.0.12?
Comment on attachment 259120 [details] [diff] [review]
for branches

approved for 1.8.0.12 and 1.8.1.4, a=dveditz for release-drivers
Attachment #259120 - Flags: approval1.8.1.4?
Attachment #259120 - Flags: approval1.8.1.4+
Attachment #259120 - Flags: approval1.8.0.12?
Attachment #259120 - Flags: approval1.8.0.12+
Keywords: fixed1.8.0.12, fixed1.8.1.4
verified fixed on the 1.8 branch using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.4pre) Gecko/2007050804 BonEcho/2.0.0.4pre. No crash with Testcase in Comment 1. Adding branch verified keyword.
Keywords: fixed1.8.1.4 → verified1.8.1.4
verified fixed on the 1.8.0 branch using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.0.12pre) Gecko/20070508 Firefox/1.5.0.12pre. No crash with
Testcase in Comment 1. Adding branch verified keyword.
Keywords: fixed1.8.0.12 → verified1.8.0.12
Group: security
Flags: in-testsuite?

Comment 10

8 years ago
crash test landed
http://hg.mozilla.org/mozilla-central/rev/c9211196a28e
Flags: in-testsuite? → in-testsuite+
Depends on: 1268050
You need to log in before you can comment on or make changes to this bug.