Closed Bug 778428 (CVE-2012-1972) Opened 13 years ago Closed 13 years ago

Heap-use-after-free in nsHTMLEditor::CollapseAdjacentTextNodes

Categories

(Core :: DOM: Editor, defect)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla17
Tracking Status
firefox14 --- wontfix
firefox15 + fixed
firefox16 + verified
firefox17 + verified
firefox-esr10 15+ fixed

People

(Reporter: inferno, Assigned: ayg)

Details

(6 keywords, Whiteboard: [asan][advisory-tracking+][qa-])

Attachments

(3 files)

Attached file Testcase
Reproduces on trunk 20120728051930 http://hg.mozilla.org/mozilla-central/rev/08428deb1e89 ================================================================= ==12811== ERROR: AddressSanitizer heap-use-after-free on address 0x7f81c5127cf0 at pc 0x7f81e8acb7f9 bp 0x7fff2669f0d0 sp 0x7fff2669f0c8 READ of size 8 at 0x7f81c5127cf0 thread T0 #0 0x7f81e8acb7f9 in nsHTMLEditor::CollapseAdjacentTextNodes(nsIDOMRange*) src/editor/libeditor/html/nsHTMLEditor.cpp:3810 #1 0x7f81e8b5aa3e in nsHTMLEditRules::AfterEditInner(nsEditor::OperationID, short) src/editor/libeditor/html/nsHTMLEditRules.cpp:451 #2 0x7f81e8b587f1 in nsHTMLEditRules::AfterEdit(nsEditor::OperationID, short) src/editor/libeditor/html/nsHTMLEditRules.cpp:376 #3 0x7f81e8ac34c5 in nsHTMLEditor::EndOperation() src/editor/libeditor/html/nsHTMLEditor.cpp:3505 #4 0x7f81e84e6554 in ~nsAutoRules src/editor/libeditor/base/nsEditorUtils.h:95 #5 0x7f81e84c4373 in ~nsAutoRules src/editor/libeditor/base/nsEditorUtils.h:97 #6 0x7f81e8a99cb7 in nsHTMLEditor::Indent(nsAString_internal const&) src/editor/libeditor/html/nsHTMLEditor.cpp:2229 #7 0x7f81e8a9b01f in non-virtual thunk to nsHTMLEditor::Indent(nsAString_internal const&) asn1cmn.c:0 #8 0x7f81ec071575 in nsOutdentCommand::DoCommand(char const*, nsISupports*) src/editor/composer/src/nsComposerCommands.cpp:519 #9 0x7f81eb8fbf9d in nsControllerCommandTable::DoCommand(char const*, nsISupports*) src/embedding/components/commandhandler/src/nsControllerCommandTable.cpp:158 #10 0x7f81eb8ce894 in nsBaseCommandController::DoCommand(char const*) src/embedding/components/commandhandler/src/nsBaseCommandController.cpp:137 #11 0x7f81eb8e6ce1 in nsCommandManager::DoCommand(char const*, nsICommandParams*, nsIDOMWindow*) src/embedding/components/commandhandler/src/nsCommandManager.cpp:238 #12 0x7f81e6e54cea in nsHTMLDocument::ExecCommand(nsAString_internal const&, bool, nsAString_internal const&, bool*) src/content/html/document/src/nsHTMLDocument.cpp:3201 #13 0x7f81e6e5765e in non-virtual thunk to nsHTMLDocument::ExecCommand(nsAString_internal const&, bool, nsAString_internal const&, bool*) asn1cmn.c:0 #14 0x7f81eee55fcb in NS_InvokeByIndex_P src/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:162 #15 0x7f81ea44923b in CallMethodHelper::Call() src/js/xpconnect/src/XPCWrappedNative.cpp:2416 #16 0x7f81ea4b05c4 in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1474 #17 0x7f81f5ae9677 in js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) src/js/src/jscntxtinlines.h:387 #18 0x7f81f5a5dbe8 in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) src/js/src/jsinterp.cpp:2405 #19 0x7f81f59def35 in js::RunScript(JSContext*, JSScript*, js::StackFrame*) src/js/src/jsinterp.cpp:302 #20 0x7f81f5ae9a99 in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) src/js/src/jsinterp.cpp:356 #21 0x7f81f540ff50 in js::Invoke(JSContext*, js::InvokeArgsGuard&, js::MaybeConstruct) src/js/src/jsinterp.h:119 #22 0x7f81f5aeebfd in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::Value*) src/js/src/jsinterp.cpp:388 #23 0x7f81f52b5bb9 in JS_CallFunctionValue src/js/src/jsapi.cpp:5698 #24 0x7f81ea3f860e in nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) src/js/xpconnect/src/XPCWrappedJSClass.cpp:1436 #25 0x7f81ea39e508 in nsXPCWrappedJS::CallMethod(unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) src/js/xpconnect/src/XPCWrappedJS.cpp:580 #26 0x7f81eee5b7d0 in PrepareAndDispatch src/xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp:121 #27 0x7f81eee58f67 in SharedStub src/xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp:0 #28 0x7f81e6256563 in nsEventListenerManager::HandleEventSubType(nsListenerStruct*, nsIDOMEventListener*, nsIDOMEvent*, nsIDOMEventTarget*, unsigned int, nsCxPusher*) src/content/events/src/nsEventListenerManager.cpp:794 #29 0x7f81e6257a2a in nsEventListenerManager::HandleEventInternal(nsPresContext*, nsEvent*, nsIDOMEvent**, nsIDOMEventTarget*, unsigned int, nsEventStatus*, nsCxPusher*) src/content/events/src/nsEventListenerManager.cpp:867 #30 0x7f81e63e6f77 in nsEventListenerManager::HandleEvent(nsPresContext*, nsEvent*, nsIDOMEvent**, nsIDOMEventTarget*, unsigned int, nsEventStatus*, nsCxPusher*) src/content/events/src/nsEventListenerManager.h:144 #31 0x7f81e63d5c16 in nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor&, unsigned int, bool, nsCxPusher*) src/content/events/src/nsEventDispatcher.cpp:184 #32 0x7f81e63d377c in nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor&, unsigned int, nsDispatchingCallback*, bool, nsCxPusher*) src/content/events/src/nsEventDispatcher.cpp:312 #33 0x7f81e63d9290 in nsEventDispatcher::Dispatch(nsISupports*, nsPresContext*, nsEvent*, nsIDOMEvent*, nsEventStatus*, nsDispatchingCallback*, nsCOMArray<nsIDOMEventTarget>*) src/content/events/src/nsEventDispatcher.cpp:633 #34 0x7f81e63dd824 in nsEventDispatcher::DispatchDOMEvent(nsISupports*, nsEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*) src/content/events/src/nsEventDispatcher.cpp:696 #35 0x7f81e57a21b3 in nsINode::DispatchEvent(nsIDOMEvent*, bool*) src/content/base/src/nsINode.cpp:1079 #36 0x7f81e52e4972 in nsContentUtils::DispatchEvent(nsIDocument*, nsISupports*, nsAString_internal const&, bool, bool, bool, bool*) src/content/base/src/nsContentUtils.cpp:3428 #37 0x7f81e52e3e06 in nsContentUtils::DispatchTrustedEvent(nsIDocument*, nsISupports*, nsAString_internal const&, bool, bool, bool*) src/content/base/src/nsContentUtils.cpp:3398 #38 0x7f81e54eca26 in nsDocument::DispatchContentLoadedEvents() src/content/base/src/nsDocument.cpp:4131 #39 0x7f81e55ad459 in nsRunnableMethodImpl<void (nsDocument::*)(), true>::Run() src/../../../dist/include/nsThreadUtils.h:349 #40 0x7f81eed56e0d in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:625 #41 0x7f81ee9e595d in NS_ProcessNextEvent_P(nsIThread*, bool) src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:220 #42 0x7f81ed97f886 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:82 #43 0x7f81ef00a76a in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:209 #44 0x7f81ef00a5b3 in MessageLoop::RunHandler() src/ipc/chromium/src/base/message_loop.cc:202 #45 0x7f81ef00a498 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:176 #46 0x7f81ece8568e in nsBaseAppShell::Run() src/widget/xpwidgets/nsBaseAppShell.cpp:165 #47 0x7f81ebad3448 in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:271 #48 0x7f81e2210820 in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:3798 #49 0x7f81e22171c2 in XREMain::XRE_main(int, char**, nsXREAppData const*) src/toolkit/xre/nsAppRunner.cpp:3875 #50 0x7f81e221a692 in XRE_main src/toolkit/xre/nsAppRunner.cpp:3951 #51 0x40c28f in do_main(int, char**) src/browser/app/nsBrowserApp.cpp:174 #52 0x409cbd in main src/browser/app/nsBrowserApp.cpp:312 #53 0x7f81ff037c4d in ?? ??:0 0x7f81c5127cf0 is located 112 bytes inside of 120-byte region [0x7f81c5127c80,0x7f81c5127cf8) freed by thread T0 here: #0 0x4a4392 in free ??:0 #1 0x7f81fbec35c3 in moz_free src/memory/mozalloc/mozalloc.cpp:49 #2 0x7f81e5971366 in ~nsTextNode src/content/base/src/nsTextNode.cpp:121 #3 0x7f81e584274d in nsNodeUtils::LastRelease(nsINode*) src/content/base/src/nsNodeUtils.cpp:252 #4 0x7f81e56a878f in nsGenericDOMDataNode::Release() src/content/base/src/nsGenericDOMDataNode.cpp:113 #5 0x7f81e5971794 in nsTextNode::Release() src/content/base/src/nsTextNode.cpp:124 #6 0x7f81e21da640 in ~nsCOMPtr_base src/../../dist/include/nsCOMPtr.h:408 #7 0x7f81e37264b6 in nsCOMPtr<nsIContent>::~nsCOMPtr() src/../../dist/include/nsCOMPtr.h:447 #8 0x7f81e3726183 in nsCOMPtr<nsIContent>::~nsCOMPtr() src/../../dist/include/nsCOMPtr.h:447 #9 0x7f81e5705272 in nsGenericElement::RemoveChildAt(unsigned int, bool) src/content/base/src/nsGenericElement.cpp:2629 #10 0x7f81e652dd23 in nsGenericHTMLElement::SetInnerHTML(nsAString_internal const&) src/content/html/content/src/nsGenericHTMLElement.cpp:1338 #11 0x7f81e666de38 in nsHTMLBodyElement::SetInnerHTML(nsAString_internal const&) src/content/html/content/src/nsHTMLBodyElement.cpp:71 #12 0x7f81ead19acd in nsIDOMHTMLElement_SetInnerHTML(JSContext*, JS::Handle<JSObject*>, JS::Handle<long>, int, JS::Value*) src/objdir-ff-asan-sym/js/xpconnect/src/dom_quickstubs.cpp:14166 #13 0x7f81f5d61d60 in js::CallJSPropertyOpSetter(JSContext*, int (*)(JSContext*, JS::Handle<JSObject*>, JS::Handle<long>, int, JS::Value*), JS::Handle<JSObject*>, JS::Handle<long>, int, JS::Value*) src/js/src/jscntxtinlines.h:463 #14 0x7f81f5d93bc7 in js::baseops::SetPropertyHelper(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, JS::Handle<long>, unsigned int, JS::Value*, int) src/js/src/jsobj.cpp:4906 #15 0x7f81f5b30601 in js::SetPropertyOperation(JSContext*, unsigned char*, JS::Value const&, JS::Value const&) src/js/src/jsinterpinlines.h:350 #16 0x7f81f5a5266f in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) src/js/src/jsinterp.cpp:2319 #17 0x7f81f59def35 in js::RunScript(JSContext*, JSScript*, js::StackFrame*) src/js/src/jsinterp.cpp:302 #18 0x7f81f5ae9a99 in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) src/js/src/jsinterp.cpp:356 #19 0x7f81f540ff50 in js::Invoke(JSContext*, js::InvokeArgsGuard&, js::MaybeConstruct) src/js/src/jsinterp.h:119 #20 0x7f81f5aeebfd in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::Value*) src/js/src/jsinterp.cpp:388 #21 0x7f81f52b5bb9 in JS_CallFunctionValue src/js/src/jsapi.cpp:5698 #22 0x7f81ea3f860e in nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) src/js/xpconnect/src/XPCWrappedJSClass.cpp:1436 #23 0x7f81ea39e508 in nsXPCWrappedJS::CallMethod(unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) src/js/xpconnect/src/XPCWrappedJS.cpp:580 #24 0x7f81eee5b7d0 in PrepareAndDispatch src/xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp:121 #25 0x7f81eee58f67 in SharedStub src/xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp:0 #26 0x7f81e6256563 in nsEventListenerManager::HandleEventSubType(nsListenerStruct*, nsIDOMEventListener*, nsIDOMEvent*, nsIDOMEventTarget*, unsigned int, nsCxPusher*) src/content/events/src/nsEventListenerManager.cpp:794 #27 0x7f81e6257a2a in nsEventListenerManager::HandleEventInternal(nsPresContext*, nsEvent*, nsIDOMEvent**, nsIDOMEventTarget*, unsigned int, nsEventStatus*, nsCxPusher*) src/content/events/src/nsEventListenerManager.cpp:867 #28 0x7f81e63e6f77 in nsEventListenerManager::HandleEvent(nsPresContext*, nsEvent*, nsIDOMEvent**, nsIDOMEventTarget*, unsigned int, nsEventStatus*, nsCxPusher*) src/content/events/src/nsEventListenerManager.h:144 #29 0x7f81e63d5c16 in nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor&, unsigned int, bool, nsCxPusher*) src/content/events/src/nsEventDispatcher.cpp:184 previously allocated by thread T0 here: #0 0x4a4452 in __interceptor_malloc ??:0 #1 0x7f81fbec3717 in moz_xmalloc src/memory/mozalloc/mozalloc.cpp:54 #2 0x7f81e5972e75 in nsTextNode::CloneDataNode(nsINodeInfo*, bool) const src/content/base/src/nsTextNode.cpp:146 #3 0x7f81e51f0364 in nsGenericDOMDataNode::Clone(nsINodeInfo*, nsINode**) const src/content/xml/content/src/../../../base/src/nsGenericDOMDataNode.h:216 #4 0x7f81e5847d53 in nsNodeUtils::CloneAndAdopt(nsINode*, bool, bool, nsNodeInfoManager*, JSContext*, JSObject*, nsCOMArray<nsINode>&, nsINode*, nsINode**) src/content/base/src/nsNodeUtils.cpp:438 #5 0x7f81e55e92fa in nsNodeUtils::CloneAndAdopt(nsINode*, bool, bool, nsNodeInfoManager*, JSContext*, JSObject*, nsCOMArray<nsINode>&, nsIDOMNode**) src/../../../../dist/include/nsNodeUtils.h:272 #6 0x7f81e5500755 in nsNodeUtils::Clone(nsINode*, bool, nsNodeInfoManager*, nsCOMArray<nsINode>&, nsIDOMNode**) src/../../../../dist/include/nsNodeUtils.h:145 #7 0x7f81e5845bf6 in nsNodeUtils::CloneNodeImpl(nsINode*, bool, bool, nsIDOMNode**) src/content/base/src/nsNodeUtils.cpp:360 #8 0x7f81e51fba6f in nsGenericDOMDataNode::CloneNode(bool, unsigned char, nsIDOMNode**) src/content/xml/content/src/../../../base/src/nsGenericDOMDataNode.h:119 #9 0x7f81e5979fff in nsTextNode::CloneNode(bool, unsigned char, nsIDOMNode**) src/content/base/src/nsTextNode.h:27 #10 0x7f81e597df6f in non-virtual thunk to nsTextNode::CloneNode(bool, unsigned char, nsIDOMNode**) asn1cmn.c:0 #11 0x7f81e58e6539 in nsRange::CloneContents(nsIDOMDocumentFragment**) src/content/base/src/nsRange.cpp:1974 #12 0x7f81ea6eee9b in nsIDOMRange_CloneContents(JSContext*, unsigned int, JS::Value*) src/objdir-ff-asan-sym/js/xpconnect/src/dom_quickstubs.cpp:18372 #13 0x7f81f5ae9677 in js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) src/js/src/jscntxtinlines.h:387 #14 0x7f81f5a5dbe8 in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) src/js/src/jsinterp.cpp:2405 #15 0x7f81f59def35 in js::RunScript(JSContext*, JSScript*, js::StackFrame*) src/js/src/jsinterp.cpp:302 #16 0x7f81f5ae9a99 in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) src/js/src/jsinterp.cpp:356 #17 0x7f81f540ff50 in js::Invoke(JSContext*, js::InvokeArgsGuard&, js::MaybeConstruct) src/js/src/jsinterp.h:119 #18 0x7f81f5aeebfd in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::Value*) src/js/src/jsinterp.cpp:388 #19 0x7f81f52b5bb9 in JS_CallFunctionValue src/js/src/jsapi.cpp:5698 #20 0x7f81ea3f860e in nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) src/js/xpconnect/src/XPCWrappedJSClass.cpp:1436 #21 0x7f81ea39e508 in nsXPCWrappedJS::CallMethod(unsigned short, XPTMethodDescriptor const*, nsXPTCMiniVariant*) src/js/xpconnect/src/XPCWrappedJS.cpp:580 #22 0x7f81eee5b7d0 in PrepareAndDispatch src/xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp:121 #23 0x7f81eee58f67 in SharedStub src/xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp:0 #24 0x7f81e6256563 in nsEventListenerManager::HandleEventSubType(nsListenerStruct*, nsIDOMEventListener*, nsIDOMEvent*, nsIDOMEventTarget*, unsigned int, nsCxPusher*) src/content/events/src/nsEventListenerManager.cpp:794 ==12811== ABORTING Stats: 220M malloced (239M for red zones) by 552524 calls Stats: 82M realloced by 23243 calls Stats: 178M freed by 294094 calls Stats: 47M really freed by 74448 calls Stats: 448M (114755 full pages) mmaped in 112 calls mmaps by size class: 8:425958; 9:57337; 10:16380; 11:16376; 12:3072; 13:2048; 14:1536; 15:384; 16:576; 17:128; 18:224; 19:112; 20:40; mallocs by size class: 8:454989; 9:54790; 10:18129; 11:16960; 12:2544; 13:2096; 14:1545; 15:345; 16:590; 17:145; 18:241; 19:112; 20:38; frees by size class: 8:215604; 9:44215; 10:14411; 11:13853; 12:1685; 13:1785; 14:1296; 15:278; 16:518; 17:116; 18:197; 19:102; 20:34; rfrees by size class: 8:64335; 9:3355; 10:2380; 11:3507; 12:193; 13:231; 14:145; 15:39; 16:164; 17:27; 18:21; 19:29; 20:22; Stats: malloc large: 536 small slow: 2323 Shadow byte and word: 0x1ff038a24f9e: fd 0x1ff038a24f98: fd fd fd fd fd fd fd fd More shadow bytes: 0x1ff038a24f78: fd fd fd fd fd fd fd fd 0x1ff038a24f80: fa fa fa fa fa fa fa fa 0x1ff038a24f88: fa fa fa fa fa fa fa fa 0x1ff038a24f90: fd fd fd fd fd fd fd fd =>0x1ff038a24f98: fd fd fd fd fd fd fd fd 0x1ff038a24fa0: fa fa fa fa fa fa fa fa 0x1ff038a24fa8: fa fa fa fa fa fa fa fa 0x1ff038a24fb0: fd fd fd fd fd fd fd fd 0x1ff038a24fb8: fd fd fd fd fd fd fd fd
Severity: normal → critical
Component: General → Editor
Product: Firefox → Core
Whiteboard: [asan]
Version: Trunk → unspecified
Attached file stack
CollapseAdjacentTextNodes stores non-refcounted nsIDOMNode* in the local 'textNodes' array. This is the stack when one of those nodes are deleted. Adding a nsAutoScriptBlocker in CollapseAdjacentTextNodes fixes the crash, but I'm not sure if that's sufficient in general. We should also analyze the code in the enclosing methods, nsHTMLEditRules::AfterEditInner etc, and make sure they can deal with script running. We crash at: result = rightTextNode->GetPreviousSibling(getter_AddRefs(prevSibOfRightNode)); with 'rightTextNode' deleted. GetPreviousSibling is virtual so this seems exploitable.
Same crash occurs on esr10 branch with s/execCommand("Outdent")/execCommand("Outdent",false,null)/ so it probably affects all branches.
$ git grep 'nsTArray<nsI' editor/ editor/libeditor/html/nsHTMLCSSUtils.cpp:840:nsHTMLCSSUtils::BuildCSSDeclarations(nsTArray<nsIAtom*> & aPropertyArray, editor/libeditor/html/nsHTMLCSSUtils.cpp:887: nsTArray<nsIAtom*>& cssPropertyArray, editor/libeditor/html/nsHTMLCSSUtils.cpp:1002: nsTArray<nsIAtom*> cssPropertyArray; editor/libeditor/html/nsHTMLCSSUtils.cpp:1036: nsTArray<nsIAtom*> cssPropertyArray; editor/libeditor/html/nsHTMLCSSUtils.cpp:1076: nsTArray<nsIAtom*> cssPropertyArray; editor/libeditor/html/nsHTMLCSSUtils.h:337: void BuildCSSDeclarations(nsTArray<nsIAtom*> & aPropertyArray, editor/libeditor/html/nsHTMLCSSUtils.h:360: nsTArray<nsIAtom*>& aPropertyArray, editor/libeditor/html/nsHTMLEditRules.cpp:7652: nsTArray<nsINode*> skipList; editor/libeditor/html/nsHTMLEditor.cpp:3772: nsTArray<nsIDOMNode*> textNodes; editor/libeditor/html/nsTableEditor.cpp:1309: nsTArray<nsIDOMElement*> spanCellList; editor/libeditor/html/nsTableEditor.cpp:2147: nsTArray<nsIDOMElement*> deleteList; The proper fix here is to store comptr's in the arrays for the last four cases.
Assignee: nobody → ayg
*Also*, we should add a script blocker in nsHTMLEditor::EndOperation I think (and verify that it doesn't break anything.) Nothing that gets called from that function expects scripts to run. :(
(In reply to Ehsan Akhgari [:ehsan] from comment #3) > The proper fix here is to store comptr's in the arrays for the last four > cases. This patch does that, but I can't reproduce the crash on m-c even without this patch. Does the crash still happen on tip? (In reply to Ehsan Akhgari [:ehsan] from comment #4) > *Also*, we should add a script blocker in nsHTMLEditor::EndOperation I think > (and verify that it doesn't break anything.) Nothing that gets called from > that function expects scripts to run. :( Should that be in this bug? Try: https://tbpl.mozilla.org/?tree=Try&rev=6b59d9707fc9
Attachment #647480 - Flags: review?(ehsan)
(In reply to :Aryeh Gregor from comment #5) > Created attachment 647480 [details] [diff] [review] > Patch, nsTArray -> nsTArray<nsCOMPtr> > > (In reply to Ehsan Akhgari [:ehsan] from comment #3) > > The proper fix here is to store comptr's in the arrays for the last four > > cases. > > This patch does that, but I can't reproduce the crash on m-c even without > this patch. Does the crash still happen on tip? That worries me. Jesse, do you test this stuff in your fuzzer? > (In reply to Ehsan Akhgari [:ehsan] from comment #4) > > *Also*, we should add a script blocker in nsHTMLEditor::EndOperation I think > > (and verify that it doesn't break anything.) Nothing that gets called from > > that function expects scripts to run. :( > > Should that be in this bug? Yes.
Attachment #647480 - Flags: review?(ehsan) → review+
Whiteboard: [asan] → [asan][Leave open]
(In reply to Ehsan Akhgari [:ehsan] from comment #4) > *Also*, we should add a script blocker in nsHTMLEditor::EndOperation I think > (and verify that it doesn't break anything.) Nothing that gets called from > that function expects scripts to run. :( This does break stuff. editor/libeditor/html/crashtests/682650-1.html asserts because it tries to fire mutation events but it's not safe. nsHTMLEditor::EndOperation() calls nsHTMLEditRules::AfterEdit() which calls nsHTMLEditRules::AfterEditInner() which calls nsHTMLEditor::CollapseAdjacentTextNodes(), which of course wants to do various DOM mutations. Tell me if you want me to pursue this, and if so how.
Whiteboard: [asan][Leave open] → [asan]
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
(In reply to :Aryeh Gregor from comment #8) > (In reply to Ehsan Akhgari [:ehsan] from comment #4) > > *Also*, we should add a script blocker in nsHTMLEditor::EndOperation I think > > (and verify that it doesn't break anything.) Nothing that gets called from > > that function expects scripts to run. :( > > This does break stuff. editor/libeditor/html/crashtests/682650-1.html > asserts because it tries to fire mutation events but it's not safe. > nsHTMLEditor::EndOperation() calls nsHTMLEditRules::AfterEdit() which calls > nsHTMLEditRules::AfterEditInner() which calls > nsHTMLEditor::CollapseAdjacentTextNodes(), which of course wants to do > various DOM mutations. Tell me if you want me to pursue this, and if so how. Sigh... No, that's probably not worth it then. Let's leave this bug as is for now.
This is sec-critical. Have we looked at uplifting this to Aurora and Beta?
Comment on attachment 647480 [details] [diff] [review] Patch, nsTArray -> nsTArray<nsCOMPtr> (In reply to Al Billings [:abillings] from comment #11) > This is sec-critical. Have we looked at uplifting this to Aurora and Beta? We should. The risk here is very low.
Attachment #647480 - Flags: approval-mozilla-beta?
Attachment #647480 - Flags: approval-mozilla-aurora?
Attachment #647480 - Flags: approval-mozilla-beta?
Attachment #647480 - Flags: approval-mozilla-beta+
Attachment #647480 - Flags: approval-mozilla-aurora?
Attachment #647480 - Flags: approval-mozilla-aurora+
Keywords: verifyme
This is also tracking for ESR10 - does the current fix land cleanly? Please nominate if so or let us know if an alternate patch is forthcoming.
Whiteboard: [asan] → [asan][advisory-tracking+]
Comment on attachment 647480 [details] [diff] [review] Patch, nsTArray -> nsTArray<nsCOMPtr> Yes, the same basic patch applies to ESR10, although one chunk doesn't apply cleanly and needs to be manually re-applied (but it's the exact same change).
Attachment #647480 - Flags: approval-mozilla-esr10?
Attachment #647480 - Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
Someone else please push to Aurora -- I don't have time to watch the tree.
Er, ESR10, not Aurora.
Alias: CVE-2012-1972
Confirmed testcase reproducible with try-server ASan build from decoder with changeset 9f3cc040e41a. Verified testcase not reproducible with: * 17.0a1: 198ca6edd0ae (debug) built on 20120823 by decoder * 16.0a2: 805e936380ab (debug) built on 20120823 by decoder qa- for Firefox 15 and ESR15 as builds are not available.
Status: RESOLVED → VERIFIED
Keywords: verifyme
Whiteboard: [asan][advisory-tracking+] → [asan][advisory-tracking+][qa-]
Group: core-security
Flags: sec-bounty+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: