Closed Bug 771994 (CVE-2012-3959) Opened 12 years ago Closed 12 years ago

Heap-use-after-free in nsRangeUpdater::SelAdjDeleteNode

Categories

(Core :: DOM: Editor, defect)

Product:
Core
Component:
DOM: Editor
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla16
Tracking Flags:
Tracking Status
firefox14 + wontfix
firefox15 + fixed
firefox16 + verified
firefox17 + verified
firefox-esr10 15+ fixed

People

(Reporter: inferno, Assigned: ayg)

References

Details

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

Crash Data

Attachments

(3 files, 2 obsolete files)

Testcase
891 bytes, text/html
Details
Make nsRangeStore refcounted
16.06 KB, patch
ehsan.akhgari
: review+
akeybl
: approval-mozilla-aurora+
akeybl
: approval-mozilla-beta+
Details | Diff | Splinter Review
Patch backported to ESR10 v2 -- compiles, pushed to try
16.60 KB, patch
ehsan.akhgari
: review+
lsblakk
: approval-mozilla-esr10+
Details | Diff | Splinter Review 
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
Component: General → Editor
Product: Firefox → Core
Keywords: testcase-wanted
Attached file Testcase — 
Looks like i forgot to attach it last night.
Keywords: testcase-wanted
Whiteboard: [asan]
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...
Keywords: testcase
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?
Attached patch Patch (v1) (obsolete) — — Splinter Review 
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.
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #640329 - Flags: review?(roc)
(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.
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...
Attachment #640329 - Flags: review?(roc)
(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
(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. :(
The test case works on nightly - just sayin' (kaboomed me during triage).
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!
Assignee: ehsan → ayg
Severity: normal → critical
Crash Signature: [@ nsRangeUpdater::SelAdjDeleteNode]
Keywords: crash
Attachment #641499 - Flags: review?(ehsan) → review+
How far back does this problem go, do we need to fix ESR or Firefox 15?
status-firefox16: --- → affected
tracking-firefox16: --- → +
Keywords: regressionwindow-wanted
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
status-firefox13: --- → affected
Flags: in-testsuite?
Hardware: x86_64 → All
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/a67b71ac0ba7
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
status-firefox16: affected → fixed
Resolution: --- → FIXED
(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...
status-firefox-esr10: --- → affected
status-firefox14: --- → affected
status-firefox15: --- → affected
tracking-firefox-esr10: --- → ?
tracking-firefox14: --- → +
tracking-firefox15: --- → +
Depends on: 776323
(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?
Depends on bug 776323 in what way? would we need that bug fix in order to take this on older branches?
(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 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.
Attachment #641499 - Flags: approval-mozilla-beta?
Attachment #641499 - Flags: approval-mozilla-aurora?
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!
Attachment #641499 - Flags: approval-mozilla-beta?
Attachment #641499 - Flags: approval-mozilla-beta+
Attachment #641499 - Flags: approval-mozilla-aurora?
Attachment #641499 - Flags: approval-mozilla-aurora+
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!
Landed https://hg.mozilla.org/releases/mozilla-beta/rev/0c9df0afa02e to fix a missing include on beta.
Attached patch Patch backported to ESR10 (obsolete) — — Splinter Review 
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.
Whiteboard: [asan] → [asan][advisory-tracking+]
Making sure it compiles is definitely going to be helpful.
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
Attachment #647472 - Attachment is obsolete: true
Attachment #651290 - Flags: review?(ehsan)
Attachment #651290 - Flags: approval-mozilla-esr10?
Attachment #651290 - Flags: review?(ehsan) → review+
Keywords: verifyme
Attachment #651290 - Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
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
status-firefox-esr10: affected → fixed
Alias: CVE-2012-3959
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
status-firefox16: fixed → verified
status-firefox17: fixed → 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: