Closed Bug 636107 Opened 9 years ago Closed 9 years ago

Crash [@ nsEditor::CreateTxnForDeleteText ]


(Core :: Editor, defect, critical)

Not set



Tracking Status
blocking2.0 --- final+


(Reporter: scoobidiver, Assigned: khuey)



(Keywords: crash, regression, Whiteboard: [sg:critical?][hardblocker][has patch])

Crash Data


(1 obsolete file)

It is a new crash signature that first appeared in 4.0b12pre/20110220.
It is #69 top crasher in 4.0b12pre over the last 3 days.

Signature	nsEditor::CreateTxnForDeleteText
UUID	ffa9cc43-ad4f-40b0-a467-418f32110222
Time 	2011-02-22 17:18:14.41817
Uptime	7451
Last Crash	137771 seconds (1.6 days) before submission
Install Age	7451 seconds (2.1 hours) since version was first installed.
Product	Firefox
Version	4.0b12pre
Build ID	20110222030357
Branch	2.0
OS Version	10.6.6 10J567
CPU	x86
CPU Info	family 6 model 23 stepping 6
Crash Address	0x68ffff00
App Notes 	Renderers: 0x21a00,0x20400

Frame 	Module 	Signature [Expand] 	Source
0 		@0x68ffff00 	
1 	XUL 	nsEditor::CreateTxnForDeleteText 	nsAutoPtr.h:969
2 	XUL 	nsEditor::CreateTxnForDeleteCharacter 	editor/libeditor/base/nsEditor.cpp:4764
3 	XUL 	nsEditor::CreateTxnForDeleteInsertionPoint 	editor/libeditor/base/nsEditor.cpp:4827
4 	XUL 	nsEditor::CreateTxnForDeleteSelection 	editor/libeditor/base/nsEditor.cpp:4719
5 	XUL 	nsEditor::DeleteSelectionImpl 	editor/libeditor/base/nsEditor.cpp:4280
6 	XUL 	nsPlaintextEditor::DeleteSelection 	editor/libeditor/text/nsPlaintextEditor.cpp:788
7 	XUL 	nsEditor::HandleKeyPressEvent 	editor/libeditor/base/nsEditor.cpp:5104
8 	XUL 	nsHTMLEditor::HandleKeyPressEvent 	editor/libeditor/html/nsHTMLEditor.cpp:691
9 	XUL 	nsEditorEventListener::KeyPress 	editor/libeditor/base/nsEditorEventListener.cpp:362
10 	XUL 	nsEventListenerManager::HandleEventInternal 	content/events/src/nsEventListenerManager.cpp:175
11 	XUL 	nsEventTargetChainItem::HandleEventTargetChain 	content/events/src/nsEventListenerManager.h:146
12 	XUL 	nsEventTargetChainItem::HandleEventTargetChain 	content/events/src/nsEventDispatcher.cpp:395
13 	XUL 	nsEventDispatcher::Dispatch 	content/events/src/nsEventDispatcher.cpp:628
14 	XUL 	PresShell::HandleEventInternal 	layout/base/nsPresShell.cpp:7066
15 	XUL 	PresShell::HandleEvent 	layout/base/nsPresShell.cpp:6813
16 	XUL 	PresShell::HandleEvent 	layout/base/nsPresShell.cpp:6492
17 	XUL 	nsViewManager::DispatchEvent 	view/src/nsViewManager.cpp:1105
18 	XUL 	HandleEvent 	view/src/nsView.cpp:161
19 	XUL 	nsChildView::DispatchEvent 	widget/src/cocoa/
20 	XUL 	nsChildView::DispatchWindowEvent 	widget/src/cocoa/
21 	XUL 	-[ChildView processKeyDownEvent:] 	widget/src/cocoa/
22 	XUL 	-[ChildView keyDown:] 	widget/src/cocoa/
23 	AppKit 	-[NSWindow sendEvent:] 	
24 	XUL 	-[ToolbarWindow sendEvent:] 	widget/src/cocoa/
25 	AppKit 	-[NSApplication sendEvent:] 	
26 	AppKit 	-[NSApplication run] 	
27 	XUL 	nsAppShell::Run 	widget/src/cocoa/
28 	XUL 	nsAppStartup::Run 	toolkit/components/startup/src/nsAppStartup.cpp:220
29 	XUL 	XRE_main 	toolkit/xre/nsAppRunner.cpp:3766
30 	firefox-bin 	main 	browser/app/nsBrowserApp.cpp:158
31 	firefox-bin 	firefox-bin@0xa05 	
32 		@0x0 	

The regression range is:
The likely culprit is bug 633709.

More reports at:
khuey is worried that this is exploitable, so marking s-g.
Group: core-security
This is bad.  The problem here is that the local variable at is not initialized.  We pick up some random value from the stack and then pass it down a couple functions where it finally gets swapped into the RefPtr at  If it happens to non-null we end up calling Release on it when the RefPtr goes out of scope.

Calling virtual methods on a garbage ptr we got from the stack is not good.
Assignee: nobody → khuey
blocking2.0: --- → ?
OS: Mac OS X → All
Hardware: x86 → All
Comment on attachment 514458 [details] [diff] [review]

Remove some more raw ptr usage by editor to make this safe.
Attachment #514458 - Flags: review?(ehsan)
(In reply to comment #2)
> We pick up some random value from the stack and then pass
> it down a couple functions where it finally gets swapped into the RefPtr

Might have been beter to use .forget(...) instead of .swap(...).
Yeah, we should do that too.
Please renominate if it turns out to not be sg:crit, but based on the previous bugs like this, we're assuming it is.
blocking2.0: ? → final+
Whiteboard: [sg:critical?][hardblocker]
Whiteboard: [sg:critical?][hardblocker] → [sg:critical?][hardblocker][has patch]
Closed: 9 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 636074
Attachment #514458 - Attachment is obsolete: true
Attachment #514458 - Flags: review?(ehsan)
Kyle moved his patch over to bug 636074, and we'll take his patch there, FWIW.
Crash Signature: [@ nsEditor::CreateTxnForDeleteText ]
You need to log in before you can comment on or make changes to this bug.