Title: [242964] trunk/Source/WebCore
- Revision
- 242964
- Author
- [email protected]
- Date
- 2019-03-14 14:09:46 -0700 (Thu, 14 Mar 2019)
Log Message
Storing a Node in Ref/RefPtr inside its destructor results in double delete
https://bugs.webkit.org/show_bug.cgi?id=195661
Reviewed by Brent Fulgham.
Set Node::m_refCount to 1 before calling its virtual destructor.
This is a security mitigation to prevent any code which ends up storing the node to Ref / RefPtr
inside the destructor, which is a programming error caught by debug assertions, from triggering
a double-delete on the same Node.
Such a code would hit the debug assertions in Node::deref() because m_inRemovedLastRefFunction
had been set to true by then.
* dom/Document.cpp:
(WebCore::Document::removedLastRef):
* dom/Document.h:
(WebCore::Document::decrementReferencingNodeCount):
* dom/Node.cpp:
(WebCore::Node::~Node):
(WebCore::Node::removedLastRef):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (242963 => 242964)
--- trunk/Source/WebCore/ChangeLog 2019-03-14 20:56:18 UTC (rev 242963)
+++ trunk/Source/WebCore/ChangeLog 2019-03-14 21:09:46 UTC (rev 242964)
@@ -1,3 +1,27 @@
+2019-03-14 Ryosuke Niwa <[email protected]>
+
+ Storing a Node in Ref/RefPtr inside its destructor results in double delete
+ https://bugs.webkit.org/show_bug.cgi?id=195661
+
+ Reviewed by Brent Fulgham.
+
+ Set Node::m_refCount to 1 before calling its virtual destructor.
+
+ This is a security mitigation to prevent any code which ends up storing the node to Ref / RefPtr
+ inside the destructor, which is a programming error caught by debug assertions, from triggering
+ a double-delete on the same Node.
+
+ Such a code would hit the debug assertions in Node::deref() because m_inRemovedLastRefFunction
+ had been set to true by then.
+
+ * dom/Document.cpp:
+ (WebCore::Document::removedLastRef):
+ * dom/Document.h:
+ (WebCore::Document::decrementReferencingNodeCount):
+ * dom/Node.cpp:
+ (WebCore::Node::~Node):
+ (WebCore::Node::removedLastRef):
+
2019-03-14 Brent Fulgham <[email protected]>
Move CommonCrypto SPI declarations to an appropriate PAL/spi header
Modified: trunk/Source/WebCore/dom/Document.cpp (242963 => 242964)
--- trunk/Source/WebCore/dom/Document.cpp 2019-03-14 20:56:18 UTC (rev 242963)
+++ trunk/Source/WebCore/dom/Document.cpp 2019-03-14 21:09:46 UTC (rev 242964)
@@ -720,6 +720,7 @@
m_inRemovedLastRefFunction = false;
m_deletionHasBegun = true;
#endif
+ m_refCount = 1; // Avoid double destruction through use of RefPtr<T>. (This is a security mitigation in case of programmer error. It will ASSERT in debug builds.)
delete this;
}
}
Modified: trunk/Source/WebCore/dom/Document.h (242963 => 242964)
--- trunk/Source/WebCore/dom/Document.h 2019-03-14 20:56:18 UTC (rev 242963)
+++ trunk/Source/WebCore/dom/Document.h 2019-03-14 21:09:46 UTC (rev 242964)
@@ -376,6 +376,7 @@
#if !ASSERT_DISABLED
m_deletionHasBegun = true;
#endif
+ m_refCount = 1; // Avoid double destruction through use of RefPtr<T>. (This is a security mitigation in case of programmer error. It will ASSERT in debug builds.)
delete this;
}
}
Modified: trunk/Source/WebCore/dom/Node.cpp (242963 => 242964)
--- trunk/Source/WebCore/dom/Node.cpp 2019-03-14 20:56:18 UTC (rev 242963)
+++ trunk/Source/WebCore/dom/Node.cpp 2019-03-14 21:09:46 UTC (rev 242964)
@@ -332,7 +332,9 @@
Node::~Node()
{
ASSERT(isMainThread());
- ASSERT(!m_refCount);
+ // We set m_refCount to 1 before calling delete to avoid double destruction through use of Ref<T>/RefPtr<T>.
+ // This is a security mitigation in case of programmer errorm (caught by a debug assertion).
+ ASSERT(m_refCount == 1);
ASSERT(m_deletionHasBegun);
ASSERT(!m_adoptionIsRequired);
@@ -2532,6 +2534,7 @@
#ifndef NDEBUG
m_deletionHasBegun = true;
#endif
+ m_refCount = 1; // Avoid double destruction through use of RefPtr<T>. (This is a security mitigation in case of programmer error. It will ASSERT in debug builds.)
delete this;
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
