Bug 730441 - Fix null-pointer crash in nsTreeContentView::SetTree(). r=tnikkel
authorMats Palmgren <matspal@gmail.com>
Tue, 23 Apr 2013 00:52:22 -0500
changeset 129558 a6639d6743db4bfbb88e25214e466243bd8ad64a
parent 129557 352ceffb0d9ebe96eb8d976789e45a4647a6b50f
child 129559 7da9a210f36594d1459d48d1edd7b79a8aa7a2ef
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewerstnikkel
bugs730441
milestone23.0a1
Bug 730441 - Fix null-pointer crash in nsTreeContentView::SetTree(). r=tnikkel
layout/xul/tree/crashtests/730441-3.xul
layout/xul/tree/crashtests/crashtests.list
layout/xul/tree/nsTreeContentView.cpp
new file mode 100644
--- /dev/null
+++ b/layout/xul/tree/crashtests/730441-3.xul
@@ -0,0 +1,38 @@
+<?xml version="1.0"?>
+<!--
+###!!! ASSERTION: You can't dereference a NULL nsCOMPtr with operator->().: 'mRawPtr != 0', file ../../../../dist/include/nsCOMPtr.h, line 796
+
+Program received signal SIGSEGV, Segmentation fault.
+0xb6b7463a in nsTreeContentView::SetTree (this=0xb0ba2510, aTree=0xaaecece0) at layout/xul/base/src/tree/src/nsTreeContentView.cpp:571
+571        boxObject->GetElement(getter_AddRefs(element));
+(gdb) bt 3
+#0  0xb6b7463a in nsTreeContentView::SetTree (this=0xb0ba2510, aTree=0xaaecece0) at layout/xul/base/src/tree/src/nsTreeContentView.cpp:571
+#1  0xb736c76f in NS_InvokeByIndex_P () at xpcom/reflect/xptcall/src/md/unix/xptcinvoke_gcc_x86_unix.cpp:69
+#2  0xb6171901 in XPCWrappedNative::CallMethod (ccx=..., mode=XPCWrappedNative::CALL_METHOD)
+    at js/src/xpconnect/src/xpcwrappednative.cpp:2722
+(More stack frames follow...)
+(gdb) list 566
+561    nsTreeContentView::SetTree(nsITreeBoxObject* aTree)
+562    {
+563      ClearRows();
+564
+565      mBoxObject = aTree;
+566
+567      if (aTree && !mRoot) {
+568        // Get our root element
+569        nsCOMPtr<nsIBoxObject> boxObject = do_QueryInterface(mBoxObject);
+570        nsCOMPtr<nsIDOMElement> element;
+571        boxObject->GetElement(getter_AddRefs(element));
+(gdb) p boxObject
+$16 = {mRawPtr = 0x0}
+
+|aTree| does not implement |nsIBoxObject|, so |do_QueryInterface(mBoxObject)|
+returns null. Then we have |null->GetElement()|.
+-->
+<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
+        onload="document.getElementById('tree').view.setTree({});">
+<tree id="tree">
+  <treechildren/>
+</tree>
+</window>
+
--- a/layout/xul/tree/crashtests/crashtests.list
+++ b/layout/xul/tree/crashtests/crashtests.list
@@ -15,8 +15,9 @@ load 409807-1.xul
 load 414170-1.xul
 load 430394-1.xul
 load 454186-1.xul
 load 479931-1.xhtml
 load 509602-1.xul
 load 601427.html
 load 730441-1.xul
 load 730441-2.xul
+load 730441-3.xul
--- a/layout/xul/tree/nsTreeContentView.cpp
+++ b/layout/xul/tree/nsTreeContentView.cpp
@@ -468,19 +468,25 @@ nsTreeContentView::GetCellText(int32_t a
 
 NS_IMETHODIMP
 nsTreeContentView::SetTree(nsITreeBoxObject* aTree)
 {
   ClearRows();
 
   mBoxObject = aTree;
 
-  if (aTree && !mRoot) {
+  MOZ_ASSERT(!mRoot, "mRoot should have been cleared out by ClearRows");
+
+  if (aTree) {
     // Get our root element
     nsCOMPtr<nsIBoxObject> boxObject = do_QueryInterface(mBoxObject);
+    if (!boxObject) {
+      mBoxObject = nullptr;
+      return NS_ERROR_INVALID_ARG;
+    }
     nsCOMPtr<nsIDOMElement> element;
     boxObject->GetElement(getter_AddRefs(element));
 
     mRoot = do_QueryInterface(element);
     NS_ENSURE_STATE(mRoot);
 
     // Add ourselves to document's observers.
     nsIDocument* document = mRoot->GetDocument();