Title: [243592] branches/safari-607-branch/Source/WebCore
- Revision
- 243592
- Author
- alanc...@apple.com
- Date
- 2019-03-27 16:52:44 -0700 (Wed, 27 Mar 2019)
Log Message
Cherry-pick r242964. rdar://problem/49359851
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):
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@242964 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: branches/safari-607-branch/Source/WebCore/ChangeLog (243591 => 243592)
--- branches/safari-607-branch/Source/WebCore/ChangeLog 2019-03-27 23:50:22 UTC (rev 243591)
+++ branches/safari-607-branch/Source/WebCore/ChangeLog 2019-03-27 23:52:44 UTC (rev 243592)
@@ -1,5 +1,58 @@
2019-03-27 Alan Coon <alanc...@apple.com>
+ Cherry-pick r242964. rdar://problem/49359851
+
+ 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):
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@242964 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-03-14 Ryosuke Niwa <rn...@webkit.org>
+
+ 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-27 Alan Coon <alanc...@apple.com>
+
Cherry-pick r243506. rdar://problem/49307987
vertexAttribPointer must restrict offset parameter
Modified: branches/safari-607-branch/Source/WebCore/dom/Document.cpp (243591 => 243592)
--- branches/safari-607-branch/Source/WebCore/dom/Document.cpp 2019-03-27 23:50:22 UTC (rev 243591)
+++ branches/safari-607-branch/Source/WebCore/dom/Document.cpp 2019-03-27 23:52:44 UTC (rev 243592)
@@ -721,6 +721,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: branches/safari-607-branch/Source/WebCore/dom/Document.h (243591 => 243592)
--- branches/safari-607-branch/Source/WebCore/dom/Document.h 2019-03-27 23:50:22 UTC (rev 243591)
+++ branches/safari-607-branch/Source/WebCore/dom/Document.h 2019-03-27 23:52:44 UTC (rev 243592)
@@ -382,6 +382,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: branches/safari-607-branch/Source/WebCore/dom/Node.cpp (243591 => 243592)
--- branches/safari-607-branch/Source/WebCore/dom/Node.cpp 2019-03-27 23:50:22 UTC (rev 243591)
+++ branches/safari-607-branch/Source/WebCore/dom/Node.cpp 2019-03-27 23:52:44 UTC (rev 243592)
@@ -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);
@@ -2537,6 +2539,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
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
