Closed Bug 646662 Opened 9 years ago Closed 9 years ago

###!!! ASSERTION: This is unsafe! Fix the caller! in nsEventDispatcher

Categories

(Core :: DOM: Core & HTML, defect, critical)

x86
Windows 7
defect
Not set
critical

Tracking

()

VERIFIED FIXED
Tracking Status
firefox5 + fixed
firefox6 + fixed
firefox7 + fixed
blocking2.0 --- -
status2.0 --- wanted
blocking1.9.2 --- .18+
status1.9.2 --- .18-fixed

People

(Reporter: nils, Assigned: smaug)

References

Details

(Keywords: verified1.9.2, Whiteboard: [sg:critical?] fixed on trunk by 650493 [qa-examined-192])

Attachments

(5 files)

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0) Gecko/20100101 Firefox/4.0
Build Identifier: 

Description:
The attached testcase triggers following assertion on debug builds:

 ###!!! ASSERTION: This is unsafe! Fix the caller!

A stack back trace at the time of the assertion is attached. Additionally exploitable looking crashes have been observed on Linux, Windows and Mac.

The reduced test case needs a few reloads to crash the browser (especially on windows). The likelihood of crashes can be increased by loading the testcase in several tabs and iframes. Regardless of the crashes, the assertion seem to happen for every reload.

Affected Versions:
Firefox 4.0
Trunk

Testcase:
The testcase is attached as an HTML file. It will crash the browser on opening after several reloads. 

Testcase Notes:
gc() triggers garbage collection. It requires Jesse's quitter extension (https://www.squarefree.com/extensions/quitter.xpi).

Stack Backtrace:

The stack during the first assertion on Linux is attached.

VulnDev reference    : vd11007

reported by nils of vulndev ltd.

Reproducible: Always
Attached file gdb output on mac
Attached file stack during assertion
Should we be popping removable script blockers before calling userdata handlers?

Can we just drop support for userdata?  :(
Status: UNCONFIRMED → NEW
Ever confirmed: true
Component: DOM: Events → DOM: Core & HTML
QA Contact: events → general
Assignee: nobody → Olli.Pettay
status2.0: --- → wanted
Whiteboard: [sg:critical?]
Can't reproduce the crash (with quitter.xpi) on 1.9.2 nor
with modified testcase (with domFuzzLite.xpi)
But the assertion is there, and that is serious enough and
fixing the cause for that could hopefully fix the crash too.
Olli, can you provide a patch that fixes the assertion alone and we'll start there?
Whiteboard: [sg:critical?] → [sg:critical?][needs response to c7 from Olli]
blocking1.9.2: --- → ?
blocking2.0: --- → -
Ollie, we're coming up to the train station and time is short for a fix that's wanted for Firefox 5. Can you give us an update here, please?
ETA end of this week
The assertion doesn't occur on trunk anymore.
Possibly fixed in Bug 650493.
building aurora for testing....
I can't get even the assertion now on 1.9.2 (with https://www.squarefree.com/extensions/quitter.xpi)

Will try on aurora once the build is ready.
Ok, I can still reproduce the assertion on aurora
As far as I see, we shouldn't have script blocker in stack when
calling AdoptNode.

On trunk the problem doesn't happen because we use nsUserDataCaller
script runner.

I doubt the patch helps with the crash (which I can't reproduce) though.
Attachment #531567 - Flags: review?(jonas)
Comment on attachment 531567 [details] [diff] [review]
Fixes the assertion

I suspect fixing bug 639648 will fix the crash.
Attachment #531567 - Flags: review?(jonas) → review+
Attachment #531567 - Flags: approval-mozilla-aurora?
blocking1.9.2: ? → .18+
Depends on: 650493
Whiteboard: [sg:critical?][needs response to c7 from Olli] → [sg:critical?][needs aurora approval] fixed on trunk by 650493?
Attachment #531567 - Flags: approval-mozilla-beta?
With the crash fix landed is it still sg:crit to get this assertion fixed? What's the risk of taking this fix so late in the Firefox 5 cycle?
Did the patch for bug 639648 fix the crash on fx5?
Attachment #531567 - Flags: approval-mozilla-aurora?
Boris, yes, that fix seems to have stopped the crash on Aurora.  Do we still need this assertion fix?
Not sure.  Check with Jonas?
Whiteboard: [sg:critical?][needs aurora approval] fixed on trunk by 650493? → [sg:critical?][needs beta approval] fixed on trunk by 650493
Comment on attachment 531567 [details] [diff] [review]
Fixes the assertion

Checked with Jonas and he feels more comfortable with taking this than not. This patch prevents us from getting into uncharted territory where exploitable things *could* be possible, though we do not at this point know of any specific ways where an actual exploit would happen.
Attachment #531567 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Whiteboard: [sg:critical?][needs beta approval] fixed on trunk by 650493 → [sg:critical?] fixed on trunk by 650493
This is fixed since bug 650493 landed. We just need to land this on the beta branch. Olli, can you do the landing?
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Yeah, I'll try to do it tomorrow.
(Sorry, I was traveling for few days)
I'm having trouble to clone mozilla-beta...
Keywords: checkin-needed
Whiteboard: [sg:critical?] fixed on trunk by 650493 → [sg:critical?] fixed on trunk by 650493 [Ollie needs help landing for Firefox 5]
I'll re-try today. the problem must have been just some networking
error.
Olli: does this patch also for for the 1.9.2 branch or do we need to do something else? code-freeze is Monday June 6
Keywords: checkin-needed
I can't reproduce the problem on 192, and the patch doesn't apply cleanly.

But I'll update the patch for 192 anyway.
Attached patch for 1.9.2Splinter Review
But to fix the real problem, we need Bug 639648
Attachment #537187 - Flags: review?(jonas)
Attachment #537187 - Flags: approval1.9.2.18?
Whiteboard: [sg:critical?] fixed on trunk by 650493 [Ollie needs help landing for Firefox 5] → [sg:critical?] fixed on trunk by 650493
We have Bug 639648 on 1.9.2...does that mean we don't need this? Or do they both need to come in?
I think we should take this, but this needs r and a.
Comment on attachment 537187 [details] [diff] [review]
for 1.9.2

Approved for 1.9.2.18, a=dveditz for release-drivers

Please land after getting jonas's OK.
Attachment #537187 - Flags: approval1.9.2.18? → approval1.9.2.18+
Since there is no STR for 1.9.2 and comment 28 says that the issue cannot be reproduced here on 1.9.2, am I correct in my belief that there is nothing for QA to do here for 1.9.2 verification?
Whiteboard: [sg:critical?] fixed on trunk by 650493 → [sg:critical?] fixed on trunk by 650493 [qa-examined-192]
Group: core-security
Mozilla/5.0 (Windows NT 6.1; rv:6.0) Gecko/20100101 Firefox/6.0
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.20) Gecko/20110803 Firefox/3.6.20
Runned the testcase and the browser did not crash.
Setting resolution to VERIFIED FIXED.
Thanks!
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.