Last Comment Bug 771994 - (CVE-2012-3959) Heap-use-after-free in nsRangeUpdater::SelAdjDeleteNode
(CVE-2012-3959)
: Heap-use-after-free in nsRangeUpdater::SelAdjDeleteNode
Status: VERIFIED FIXED
[asan][advisory-tracking+][qa-]
: crash, sec-critical, testcase
Product: Core
Classification: Components
Component: Editor (show other bugs)
: Trunk
: All All
: -- critical (vote)
: mozilla16
Assigned To: Aryeh Gregor (:ayg) (away until October 25)
:
:
Mentors:
Depends on: 776323
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-09 01:51 PDT by Abhishek Arya
Modified: 2014-07-24 13:43 PDT (History)
11 users (show)
rforbes: sec‑bounty+
ayg: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
wontfix
+
fixed
+
verified
+
verified
15+
fixed


Attachments
Testcase (891 bytes, text/html)
2012-07-09 10:39 PDT, Abhishek Arya
no flags Details
Patch (v1) (22.00 KB, patch)
2012-07-09 12:58 PDT, :Ehsan Akhgari
no flags Details | Diff | Splinter Review
Make nsRangeStore refcounted (16.06 KB, patch)
2012-07-12 08:55 PDT, Aryeh Gregor (:ayg) (away until October 25)
ehsan: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review
Patch backported to ESR10 (16.00 KB, patch)
2012-07-31 02:18 PDT, Aryeh Gregor (:ayg) (away until October 25)
no flags Details | Diff | Splinter Review
Patch backported to ESR10 v2 -- compiles, pushed to try (16.60 KB, patch)
2012-08-13 01:55 PDT, Aryeh Gregor (:ayg) (away until October 25)
ehsan: review+
lukasblakk+bugs: approval‑mozilla‑esr10+
Details | Diff | Splinter Review

Description Abhishek Arya 2012-07-09 01:51:30 PDT
Reproduces on trunk, testcase coming soon. Below is a fully symbolized stack.
20120707214206
http://hg.mozilla.org/mozilla-central/rev/9533b40ff28b

=================================================================
==21914== ERROR: AddressSanitizer heap-use-after-free on address 0x7f4120fac130 at pc 0x7f414d5a0c33 bp 0x7fff10d82d30 sp 0x7fff10d82d28
READ of size 8 at 0x7f4120fac130 thread T0
    #0 0x7f414d5a0c33 in nsCOMPtr<nsIDOMNode>::get() const firefox/src/modules/zlib/src/inffast.c:0
    #1 0x7f41521a148e in nsRangeUpdater::SelAdjDeleteNode(nsIDOMNode*) firefox/src/editor/libeditor/base/nsSelectionState.cpp:271
    #2 0x7f41521d8818 in DeleteNodeTxn::DoTransaction() firefox/src/editor/libeditor/base/DeleteNodeTxn.cpp:74
    #3 0x7f41555503b9 in nsTransactionItem::DoTransaction() firefox/src/editor/txmgr/src/nsTransactionItem.cpp:178
    #4 0x7f4155574d8e in nsTransactionManager::BeginTransaction(nsITransaction*) firefox/src/editor/txmgr/src/nsTransactionManager.cpp:729
    #5 0x7f4155562d25 in nsTransactionManager::DoTransaction(nsITransaction*) firefox/src/editor/txmgr/src/nsTransactionManager.cpp:74
    #6 0x7f41520afcf3 in nsEditor::DoTransaction(nsITransaction*) firefox/src/editor/libeditor/base/nsEditor.cpp:689
    #7 0x7f41520cc7bd in nsEditor::DeleteNode(nsINode*) firefox/src/editor/libeditor/base/nsEditor.cpp:1492
    #8 0x7f41520cbe8b in nsEditor::DeleteNode(nsIDOMNode*) firefox/src/editor/libeditor/base/nsEditor.cpp:1476
    #9 0x7f41526457ee in nsHTMLEditor::DeleteNode(nsIDOMNode*) firefox/src/editor/libeditor/html/nsHTMLEditor.cpp:3226
    #10 0x7f4152082664 in nsTextEditRules::WillInsert(nsISelection*, bool*) firefox/src/editor/libeditor/text/nsTextEditRules.cpp:324
    #11 0x7f415274d1fc in nsHTMLEditRules::WillInsert(nsISelection*, bool*) firefox/src/editor/libeditor/html/nsHTMLEditRules.cpp:1171
    #12 0x7f41526ff9bc in nsHTMLEditRules::WillInsertText(nsEditor::OperationID, mozilla::Selection*, bool*, bool*, nsAString_internal const*, nsAString_internal*, int) firefox/src/editor/libeditor/html/nsHTMLEditRules.cpp:1264
    #13 0x7f41526fbd14 in nsHTMLEditRules::WillDoAction(mozilla::Selection*, nsRulesInfo*, bool*, bool*) firefox/src/editor/libeditor/html/nsHTMLEditRules.cpp:570
    #14 0x7f4152049777 in nsPlaintextEditor::InsertText(nsAString_internal const&) firefox/src/editor/libeditor/text/nsPlaintextEditor.cpp:700
    #15 0x7f415204a2ef in non-virtual thunk to nsPlaintextEditor::InsertText(nsAString_internal const&) firefox/src/modules/zlib/src/inffast.c:0
    #16 0x7f415216b714 in nsInsertPlaintextCommand::DoCommandParams(char const*, nsICommandParams*, nsISupports*) firefox/src/editor/libeditor/base/nsEditorCommands.cpp:834
    #17 0x7f41553ebcbc in nsControllerCommandTable::DoCommandParams(char const*, nsICommandParams*, nsISupports*) firefox/src/embedding/components/commandhandler/src/nsControllerCommandTable.cpp:175
    #18 0x7f41553be57f in nsBaseCommandController::DoCommandWithParams(char const*, nsICommandParams*) firefox/src/embedding/components/commandhandler/src/nsBaseCommandController.cpp:153
    #19 0x7f41553be8b7 in non-virtual thunk to nsBaseCommandController::DoCommandWithParams(char const*, nsICommandParams*) firefox/src/modules/zlib/src/inffast.c:0
    #20 0x7f41553d6061 in nsCommandManager::DoCommand(char const*, nsICommandParams*, nsIDOMWindow*) firefox/src/embedding/components/commandhandler/src/nsCommandManager.cpp:236
    #21 0x7f4150a232f4 in nsHTMLDocument::ExecCommand(nsAString_internal const&, bool, nsAString_internal const&, bool*) firefox/src/content/html/document/src/nsHTMLDocument.cpp:3218
    #22 0x7f4150a253ee in non-virtual thunk to nsHTMLDocument::ExecCommand(nsAString_internal const&, bool, nsAString_internal const&, bool*) firefox/src/modules/zlib/src/inffast.c:0
    #23 0x7f415880dbfa in NS_InvokeByIndex_P firefox/src/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:161
    #24 0x7f4153f8a56b in CallMethodHelper::Call() firefox/src/js/xpconnect/src/XPCWrappedNative.cpp:2405
    #25 0x7f4153ff17d4 in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) firefox/src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1474
    #26 0x7f415db3664d in js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) firefox/src/js/src/jscntxtinlines.h:400
    #27 0x7f415d4c9040 in js::Invoke(JSContext*, js::InvokeArgsGuard&, js::MaybeConstruct) firefox/src/js/src/jsinterp.h:119
    #28 0x7f415db3bc0d in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::Value*) firefox/src/js/src/jsinterp.cpp:382
    #29 0x7f415df7bef9 in js::IndirectProxyHandler::call(JSContext*, JSObject*, unsigned int, JS::Value*) firefox/src/js/src/jsproxy.cpp:441
    #30 0x7f415e71c5e1 in js::DirectWrapper::call(JSContext*, JSObject*, unsigned int, JS::Value*) firefox/src/js/src/jswrapper.cpp:303
    #31 0x7f415e731179 in js::CrossCompartmentWrapper::call(JSContext*, JSObject*, unsigned int, JS::Value*) firefox/src/js/src/jswrapper.cpp:699
    #32 0x7f415e731985 in non-virtual thunk to js::CrossCompartmentWrapper::call(JSContext*, JSObject*, unsigned int, JS::Value*) ??:0
    #33 0x7f415dfb13a1 in js::Proxy::call(JSContext*, JSObject*, unsigned int, JS::Value*) firefox/src/js/src/jsproxy.cpp:1134
    #34 0x7f415dfc9b2c in proxy_Call(JSContext*, unsigned int, JS::Value*) firefox/src/js/src/jsproxy.cpp:1657
    #35 0x7f415db361f5 in js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) firefox/src/js/src/jscntxtinlines.h:400
    #36 0x7f415daa9268 in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) firefox/src/js/src/jsinterp.cpp:2465
    #37 0x7f415da2d777 in js::RunScript(JSContext*, JSScript*, js::StackFrame*) firefox/src/js/src/jsinterp.cpp:299
    #38 0x7f415db434bd in js::ExecuteKernel(JSContext*, JSScript*, JSObject&, JS::Value const&, js::ExecuteType, js::StackFrame*, JS::Value*) firefox/src/js/src/jsinterp.cpp:482
    #39 0x7f415db451c0 in js::Execute(JSContext*, JSScript*, JSObject&, JS::Value*) firefox/src/js/src/jsinterp.cpp:519
    #40 0x7f415d36b011 in EvaluateUCScriptForPrincipalsCommon(JSContext*, JSObject*, JSPrincipals*, JSPrincipals*, unsigned short const*, unsigned int, char const*, unsigned int, JS::Value*, JSVersion) firefox/src/js/src/jsapi.cpp:5370
    #41 0x7f415d36cf4c in JS_EvaluateUCScriptForPrincipalsVersionOrigin firefox/src/js/src/jsapi.cpp:5407
    #42 0x7f41511cf74f in nsJSContext::EvaluateString(nsAString_internal const&, JSObject*, nsIPrincipal*, nsIPrincipal*, char const*, unsigned int, JSVersion, nsAString_internal*, bool*) firefox/src/dom/base/nsJSEnvironment.cpp:1466
    #43 0x7f415137eb5e in nsGlobalWindow::RunTimeoutHandler(nsTimeout*, nsIScriptContext*) firefox/src/dom/base/nsGlobalWindow.cpp:9519
    #44 0x7f4151333ee2 in nsGlobalWindow::RunTimeout(nsTimeout*) firefox/src/dom/base/nsGlobalWindow.cpp:9783
    #45 0x7f415137cd6b in nsGlobalWindow::TimerCallback(nsITimer*, void*) firefox/src/dom/base/nsGlobalWindow.cpp:10055
    #46 0x7f4158748a42 in nsTimerImpl::Fire() firefox/src/xpcom/threads/nsTimerImpl.cpp:474
    #47 0x7f415874a67c in nsTimerEvent::Run() firefox/src/xpcom/threads/nsTimerImpl.cpp:558
    #48 0x7f415870cccd in nsThread::ProcessNextEvent(bool, bool*) firefox/src/xpcom/threads/nsThread.cpp:625
    #49 0x7f415839c01d in NS_ProcessNextEvent_P(nsIThread*, bool) firefox/src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:217
    #50 0x7f415743d226 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) firefox/src/ipc/glue/MessagePump.cpp:82
    #51 0x7f41589c084a in MessageLoop::RunInternal() firefox/src/ipc/chromium/src/base/message_loop.cc:209
    #52 0x7f41589c0693 in MessageLoop::RunHandler() firefox/src/ipc/chromium/src/base/message_loop.cc:202
    #53 0x7f41589c0578 in MessageLoop::Run() firefox/src/ipc/chromium/src/base/message_loop.cc:176
    #54 0x7f415697542e in nsBaseAppShell::Run() firefox/src/widget/xpwidgets/nsBaseAppShell.cpp:165
    #55 0x7f41555c0668 in nsAppStartup::Run() firefox/src/toolkit/components/startup/nsAppStartup.cpp:257
    #56 0x7f414bdfd280 in XREMain::XRE_mainRun() firefox/src/toolkit/xre/nsAppRunner.cpp:3787
    #57 0x7f414be03c22 in XREMain::XRE_main(int, char**, nsXREAppData const*) firefox/src/toolkit/xre/nsAppRunner.cpp:3864
    #58 0x7f414be070f2 in XRE_main firefox/src/toolkit/xre/nsAppRunner.cpp:3940
    #59 0x40c28f in do_main(int, char**) firefox/src/browser/app/nsBrowserApp.cpp:160
    #60 0x409cbd in main firefox/src/browser/app/nsBrowserApp.cpp:330
    #61 0x7f4166a4fc4d in ?? ??:0
0x7f4120fac130 is located 176 bytes inside of 1120-byte region [0x7f4120fac080,0x7f4120fac4e0)
freed by thread T0 here:
    #0 0x4a4392 in free ??:0
    #1 0x7f41638dd5c3 in moz_free firefox/src/memory/mozalloc/mozalloc.cpp:49
    #2 0x7f41526d8516 in ~nsHTMLEditRules firefox/src/editor/libeditor/html/nsHTMLEditRules.cpp:198
    #3 0x7f415206b677 in nsTextEditRules::Release() firefox/src/editor/libeditor/text/nsTextEditRules.cpp:90
    #4 0x7f41526d8c34 in nsHTMLEditRules::Release() firefox/src/editor/libeditor/html/nsHTMLEditRules.cpp:205
    #5 0x7f414bdc7250 in ~nsCOMPtr_base firefox/src/../../dist/include/nsCOMPtr.h:408
    #6 0x7f4152067456 in nsCOMPtr<nsIEditRules>::~nsCOMPtr() firefox/src/../../../dist/include/nsCOMPtr.h:447
    #7 0x7f41520341a3 in nsCOMPtr<nsIEditRules>::~nsCOMPtr() firefox/src/../../../dist/include/nsCOMPtr.h:447
    #8 0x7f4152047fee in nsPlaintextEditor::DeleteSelection(short, short) firefox/src/editor/libeditor/text/nsPlaintextEditor.cpp:670
    #9 0x7f41521622b8 in nsDeleteCommand::DoCommand(char const*, nsISupports*) firefox/src/editor/libeditor/base/nsEditorCommands.cpp:585
    #10 0x7f41553eb40d in nsControllerCommandTable::DoCommand(char const*, nsISupports*) firefox/src/embedding/components/commandhandler/src/nsControllerCommandTable.cpp:158
    #11 0x7f41553bdd14 in nsBaseCommandController::DoCommand(char const*) firefox/src/embedding/components/commandhandler/src/nsBaseCommandController.cpp:137
    #12 0x7f41553d6151 in nsCommandManager::DoCommand(char const*, nsICommandParams*, nsIDOMWindow*) firefox/src/embedding/components/commandhandler/src/nsCommandManager.cpp:238
    #13 0x7f4150a22a7a in nsHTMLDocument::ExecCommand(nsAString_internal const&, bool, nsAString_internal const&, bool*) firefox/src/content/html/document/src/nsHTMLDocument.cpp:3200
    #14 0x7f4150a253ee in non-virtual thunk to nsHTMLDocument::ExecCommand(nsAString_internal const&, bool, nsAString_internal const&, bool*) firefox/src/modules/zlib/src/inffast.c:0
    #15 0x7f415880dbfa in NS_InvokeByIndex_P firefox/src/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:161
    #16 0x7f4153f8a56b in CallMethodHelper::Call() firefox/src/js/xpconnect/src/XPCWrappedNative.cpp:2405
    #17 0x7f4153ff17d4 in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) firefox/src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1474
    #18 0x7f415db3664d in js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) firefox/src/js/src/jscntxtinlines.h:400
    #19 0x7f415daa9268 in js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) firefox/src/js/src/jsinterp.cpp:2465
    #20 0x7f415da2d777 in js::RunScript(JSContext*, JSScript*, js::StackFrame*) firefox/src/js/src/jsinterp.cpp:299
    #21 0x7f415db434bd in js::ExecuteKernel(JSContext*, JSScript*, JSObject&, JS::Value const&, js::ExecuteType, js::StackFrame*, JS::Value*) firefox/src/js/src/jsinterp.cpp:482
    #22 0x7f415db451c0 in js::Execute(JSContext*, JSScript*, JSObject&, JS::Value*) firefox/src/js/src/jsinterp.cpp:519
    #23 0x7f415d36b011 in EvaluateUCScriptForPrincipalsCommon(JSContext*, JSObject*, JSPrincipals*, JSPrincipals*, unsigned short const*, unsigned int, char const*, unsigned int, JS::Value*, JSVersion) firefox/src/js/src/jsapi.cpp:5370
    #24 0x7f415d36cf4c in JS_EvaluateUCScriptForPrincipalsVersionOrigin firefox/src/js/src/jsapi.cpp:5407
    #25 0x7f41511cf74f in nsJSContext::EvaluateString(nsAString_internal const&, JSObject*, nsIPrincipal*, nsIPrincipal*, char const*, unsigned int, JSVersion, nsAString_internal*, bool*) firefox/src/dom/base/nsJSEnvironment.cpp:1466
    #26 0x7f415137eb5e in nsGlobalWindow::RunTimeoutHandler(nsTimeout*, nsIScriptContext*) firefox/src/dom/base/nsGlobalWindow.cpp:9519
    #27 0x7f4151333ee2 in nsGlobalWindow::RunTimeout(nsTimeout*) firefox/src/dom/base/nsGlobalWindow.cpp:9783
    #28 0x7f415137cd6b in nsGlobalWindow::TimerCallback(nsITimer*, void*) firefox/src/dom/base/nsGlobalWindow.cpp:10055
    #29 0x7f4158748a42 in nsTimerImpl::Fire() firefox/src/xpcom/threads/nsTimerImpl.cpp:474
previously allocated by thread T0 here:
    #0 0x4a4452 in __interceptor_malloc ??:0
    #1 0x7f41638dd717 in moz_xmalloc firefox/src/memory/mozalloc/mozalloc.cpp:54
    #2 0x7f41525eab1b in nsHTMLEditor::InitRules() firefox/src/editor/libeditor/html/nsHTMLEditor.cpp:486
    #3 0x7f4152037a60 in nsPlaintextEditor::EndEditorInit() firefox/src/editor/libeditor/text/nsPlaintextEditor.cpp:186
    #4 0x7f41520689ce in ~nsAutoEditInitRulesTrigger firefox/src/editor/libeditor/text/nsTextEditUtils.cpp:85
    #5 0x7f41525e58b4 in nsHTMLEditor::Init(nsIDOMDocument*, nsIContent*, nsISelectionController*, unsigned int) firefox/src/editor/libeditor/html/nsHTMLEditor.cpp:287
    #6 0x7f4155babb31 in nsEditingSession::SetupEditorOnWindow(nsIDOMWindow*) firefox/src/editor/composer/src/nsEditingSession.cpp:459
    #7 0x7f4155ba124e in nsEditingSession::MakeWindowEditable(nsIDOMWindow*, char const*, bool, bool, bool) firefox/src/editor/composer/src/nsEditingSession.cpp:169
    #8 0x7f41509f2448 in nsHTMLDocument::EditingStateChanged() firefox/src/content/html/document/src/nsHTMLDocument.cpp:2679
    #9 0x7f4150a16e77 in nsHTMLDocument::MaybeEditingStateChanged() firefox/src/content/html/document/src/nsHTMLDocument.cpp:2326
    #10 0x7f4150a17648 in nsHTMLDocument::EndUpdate(unsigned int) firefox/src/content/html/document/src/nsHTMLDocument.cpp:2339
    #11 0x7f41524491b1 in nsHtml5TreeOpExecutor::EndDocUpdate() firefox/src/parser/html/nsHtml5TreeOpExecutor.h:265
    #12 0x7f41524487b7 in nsHtml5TreeOpExecutor::DidBuildModel(bool) firefox/src/parser/html/nsHtml5TreeOpExecutor.cpp:134
    #13 0x7f4152431d6f in nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor*, nsIContent**) firefox/src/parser/html/nsHtml5TreeOperation.cpp:621
    #14 0x7f415244ca06 in nsHtml5TreeOpExecutor::RunFlushLoop() firefox/src/parser/html/nsHtml5TreeOpExecutor.cpp:566
    #15 0x7f41524889c6 in nsHtml5ExecutorFlusher::Run() firefox/src/parser/html/nsHtml5StreamParser.cpp:127
    #16 0x7f415870cccd in nsThread::ProcessNextEvent(bool, bool*) firefox/src/xpcom/threads/nsThread.cpp:625
    #17 0x7f415839c01d in NS_ProcessNextEvent_P(nsIThread*, bool) firefox/src/objdir-ff-asan-sym/xpcom/build/nsThreadUtils.cpp:217
    #18 0x7f415743d226 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) firefox/src/ipc/glue/MessagePump.cpp:82
    #19 0x7f41589c084a in MessageLoop::RunInternal() firefox/src/ipc/chromium/src/base/message_loop.cc:209
    #20 0x7f41589c0693 in MessageLoop::RunHandler() firefox/src/ipc/chromium/src/base/message_loop.cc:202
    #21 0x7f41589c0578 in MessageLoop::Run() firefox/src/ipc/chromium/src/base/message_loop.cc:176
    #22 0x7f415697542e in nsBaseAppShell::Run() firefox/src/widget/xpwidgets/nsBaseAppShell.cpp:165
    #23 0x7f41555c0668 in nsAppStartup::Run() firefox/src/toolkit/components/startup/nsAppStartup.cpp:257
    #24 0x7f414bdfd280 in XREMain::XRE_mainRun() firefox/src/toolkit/xre/nsAppRunner.cpp:3787
==21914== ABORTING
Stats: 233M malloced (294M for red zones) by 765806 calls
Stats: 59M realloced by 49108 calls
Stats: 192M freed by 507438 calls
Stats: 71M really freed by 191258 calls
Stats: 464M (118844 full pages) mmaped in 116 calls
  mmaps   by size class: 8:491490; 9:57337; 10:36855; 11:20470; 12:4096; 13:3584; 14:1792; 15:384; 16:640; 17:192; 18:208; 19:48; 20:16;
  mallocs by size class: 8:613146; 9:72863; 10:44103; 11:23815; 12:4500; 13:3975; 14:1770; 15:446; 16:685; 17:216; 18:228; 19:46; 20:13;
  frees   by size class: 8:377324; 9:59823; 10:40191; 11:20179; 12:3343; 13:3663; 14:1532; 15:386; 16:573; 17:196; 18:176; 19:42; 20:10;
  rfrees  by size class: 8:139644; 9:27360; 10:10215; 11:10492; 12:1071; 13:679; 14:1121; 15:194; 16:366; 17:75; 18:35; 19:5; 20:1;
Stats: malloc large: 503 small slow: 3318
Shadow byte and word:
  0x1fe8241f5826: fd
  0x1fe8241f5820: fd fd fd fd fd fd fd fd
More shadow bytes:
  0x1fe8241f5800: fa fa fa fa fa fa fa fa
  0x1fe8241f5808: fa fa fa fa fa fa fa fa
  0x1fe8241f5810: fd fd fd fd fd fd fd fd
  0x1fe8241f5818: fd fd fd fd fd fd fd fd
=>0x1fe8241f5820: fd fd fd fd fd fd fd fd
  0x1fe8241f5828: fd fd fd fd fd fd fd fd
  0x1fe8241f5830: fd fd fd fd fd fd fd fd
  0x1fe8241f5838: fd fd fd fd fd fd fd fd
  0x1fe8241f5840: fd fd fd fd fd fd fd fd
Comment 1 Abhishek Arya 2012-07-09 10:39:42 PDT
Created attachment 640276 [details]
Testcase

Looks like i forgot to attach it last night.
Comment 2 :Ehsan Akhgari 2012-07-09 11:45:50 PDT
The first problem that I saw here was that the document.write(">") call triggers <http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDocument.cpp#3419>.  I'm not sure what that write call means...
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2012-07-09 12:32:54 PDT
Oh, funtimes.  So we have a root element already from the "document.appendChild(a);" and then the parser tries to add another one....

Henri, what does the spec say about what should happen in this case?
Comment 4 :Ehsan Akhgari 2012-07-09 12:58:50 PDT
Created attachment 640329 [details] [diff] [review]
Patch (v1)

nsRangeUpdater uses Chance-based Memory Management (TM).  That's sub-optimal!  I have changed it to own the values that it stores.  This fixes the use-after-free here, but there are a bunch of assertions after it resulting from the fact that our editor code is totally unable to handle the anonymous content that <video> has, but that's nothing new.
Comment 5 :Ehsan Akhgari 2012-07-09 13:05:57 PDT
(In reply to Boris Zbarsky (:bz) from comment #3)
> Oh, funtimes.  So we have a root element already from the
> "document.appendChild(a);" and then the parser tries to add another one....
> 
> Henri, what does the spec say about what should happen in this case?

Note that this is unrelated to the use after free here, I just noted it here since I'm not sure if we should attach this testcase to another bug, but this is really a separate bug.
Comment 6 :Ehsan Akhgari 2012-07-09 13:10:56 PDT
https://tbpl.mozilla.org/?tree=Try&rev=f22d9b6a28ce
Comment 7 :Ehsan Akhgari 2012-07-09 15:52:00 PDT
There's a bunch of "tried to register an already registered range" assertions with this patch.  I don't really think that check makes any sense.  I'm experimenting with taking it out...
Comment 8 :Ehsan Akhgari 2012-07-09 16:25:23 PDT
(In reply to Ehsan Akhgari [:ehsan] from comment #7)
> There's a bunch of "tried to register an already registered range"
> assertions with this patch.  I don't really think that check makes any
> sense.  I'm experimenting with taking it out...

https://tbpl.mozilla.org/?tree=Try&rev=2bf1b3b41ce7
Comment 9 :Ehsan Akhgari 2012-07-10 09:34:36 PDT
(In reply to Ehsan Akhgari [:ehsan] from comment #8)
> (In reply to Ehsan Akhgari [:ehsan] from comment #7)
> > There's a bunch of "tried to register an already registered range"
> > assertions with this patch.  I don't really think that check makes any
> > sense.  I'm experimenting with taking it out...
> 
> https://tbpl.mozilla.org/?tree=Try&rev=2bf1b3b41ce7

Ah, seems like I pushed the same patch again. :(
Comment 10 David Bolter [:davidb] 2012-07-11 10:15:55 PDT
The test case works on nightly - just sayin' (kaboomed me during triage).
Comment 11 :Ehsan Akhgari 2012-07-11 13:04:54 PDT
So I gave this a bit more thought.  nsRangeUpdater actually relies on being able to modify the entries it stores in mArray, and therefore my patch breaks all sorts of things.

So I now think that the correct way to fix this bug would be to make nsRangeStore ref-counted, and convert nsRangeUpdater::mArray into a nsTArray<nsRefPtr<nsRangeStore> >.

Aryeh would you mind looking into that?  I have a pretty good idea on what needs to happen so please ping me if you hit any problems.  Thanks!
Comment 12 Aryeh Gregor (:ayg) (away until October 25) 2012-07-12 08:55:32 PDT
Created attachment 641499 [details] [diff] [review]
Make nsRangeStore refcounted

https://tbpl.mozilla.org/?tree=Try&rev=deb985631a0a
Comment 13 Daniel Veditz [:dveditz] 2012-07-12 13:38:14 PDT
How far back does this problem go, do we need to fix ESR or Firefox 15?
Comment 14 Aryeh Gregor (:ayg) (away until October 25) 2012-07-12 23:32:46 PDT
The test case crashes Firefox 13 for me.  (I accidentally submitted a crash report, oh well.)  Marking in-testsuite? to remind us to check in the test after this bug is made public.

https://hg.mozilla.org/integration/mozilla-inbound/rev/a67b71ac0ba7
Comment 15 Ed Morley [:emorley] 2012-07-13 05:33:52 PDT
https://hg.mozilla.org/mozilla-central/rev/a67b71ac0ba7
Comment 16 :Ehsan Akhgari 2012-07-13 07:23:53 PDT
(In reply to Daniel Veditz [:dveditz] from comment #13)
> How far back does this problem go, do we need to fix ESR or Firefox 15?

Yeah it affects all branches.  Even if this specific test case doesn't crash a build, others will, since basically the lifetime of nsRangeUpdater objects was not really managed at all before this patch, and it includes members with v-tables...
Comment 17 Alex Keybl [:akeybl] 2012-07-26 17:34:56 PDT
(In reply to :Aryeh Gregor from comment #14)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/a67b71ac0ba7

Would you mind nominating for uplift to FF15 if deemed low risk?
Comment 18 Daniel Veditz [:dveditz] 2012-07-26 17:35:37 PDT
Depends on bug 776323 in what way? would we need that bug fix in order to take this on older branches?
Comment 19 :Ehsan Akhgari 2012-07-26 19:34:44 PDT
(In reply to Daniel Veditz [:dveditz] from comment #18)
> Depends on bug 776323 in what way? would we need that bug fix in order to
> take this on older branches?

Bug 776323 is a regression from this patch.  We need to take that on Aurora and Beta as well.
Comment 20 :Ehsan Akhgari 2012-07-26 19:36:30 PDT
Comment on attachment 641499 [details] [diff] [review]
Make nsRangeStore refcounted

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: sec-critical
Testing completed (on m-c, etc.): baked on m-c, and bug 776323 which is a regression from this has also been fixed there (this needs to be approved together with that bug.)
Risk to taking this patch (and alternatives if risky): I think it's sane to take this patch on branches.
String or UUID changes made by this patch: none.
Comment 21 Alex Keybl [:akeybl] 2012-07-27 16:49:12 PDT
Comment on attachment 641499 [details] [diff] [review]
Make nsRangeStore refcounted

[Triage Comment]
If landed before Tuesday, this will make it into beta 3 (which will give us time to react to any new regressions). Approved for branches.

Please also prepare a patch for the ESR10 branch. Thanks!
Comment 22 :Ehsan Akhgari 2012-07-30 10:49:37 PDT
This landed before the uplift, so doesn't need to land on Aurora again.

Aryeh, could you please prepare this patch for ESR10 as well?  Thanks!
Comment 24 :Ehsan Akhgari 2012-07-30 11:03:34 PDT
Landed https://hg.mozilla.org/releases/mozilla-beta/rev/0c9df0afa02e to fix a missing include on beta.
Comment 25 Aryeh Gregor (:ayg) (away until October 25) 2012-07-31 02:18:39 PDT
Created attachment 647472 [details] [diff] [review]
Patch backported to ESR10

There was an API change in nsRangeStore that caused slight conflict, but I didn't have to change anything significant.  I didn't try compiling this or pushing to try, though -- let me know if I should do that.
Comment 26 :Ehsan Akhgari 2012-08-09 12:45:57 PDT
Making sure it compiles is definitely going to be helpful.
Comment 27 Aryeh Gregor (:ayg) (away until October 25) 2012-08-13 01:55:50 PDT
Created attachment 651290 [details] [diff] [review]
Patch backported to ESR10 v2 -- compiles, pushed to try

This compiles on localhost.  I had to add an extra header to nsSelectionState.h for the use of nsRefPtr, in addition to the minor conflicts noted in a previous comment.  Try: https://tbpl.mozilla.org/?tree=Try&rev=b32aa36f4368
Comment 28 Aryeh Gregor (:ayg) (away until October 25) 2012-08-17 04:43:36 PDT
The try had lots of red, but Ed confirmed that that's not meaningful due to build config changes, so I went ahead and pushed:

https://hg.mozilla.org/releases/mozilla-esr10/rev/b082369c036a
Comment 29 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-08-23 15:36:00 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.