Title: [193915] branches/safari-601.1.46-branch/Source/WebCore
- Revision
- 193915
- Author
- [email protected]
- Date
- 2015-12-10 11:18:35 -0800 (Thu, 10 Dec 2015)
Log Message
Merge r192758. rdar://problem/23814314
Modified Paths
Diff
Modified: branches/safari-601.1.46-branch/Source/WebCore/ChangeLog (193914 => 193915)
--- branches/safari-601.1.46-branch/Source/WebCore/ChangeLog 2015-12-10 18:54:58 UTC (rev 193914)
+++ branches/safari-601.1.46-branch/Source/WebCore/ChangeLog 2015-12-10 19:18:35 UTC (rev 193915)
@@ -1,3 +1,43 @@
+2015-12-10 Matthew Hanson <[email protected]>
+
+ Merge r192758. rdar://problem/23814314
+
+ 2015-11-23 David Kilzer <[email protected]>
+
+ Hardening against CSSSelector double frees
+ <http://webkit.org/b/56124>
+ <rdar://problem/9119036>
+
+ Reviewed by Antti Koivisto.
+
+ Add some security assertions to catch this issue if it ever
+ happens in Debug builds, and make changes in
+ CSSSelector::~CSSSelector() and
+ CSSSelectorList::deleteSelectors() to prevent obvious issues if
+ they're ever called twice in Release builds.
+
+ No new tests because we don't know how to reproduce this.
+
+ * css/CSSSelector.cpp:
+ (WebCore::CSSSelector::CSSSelector): Initialize
+ m_destructorHasBeenCalled.
+ * css/CSSSelector.h:
+ (WebCore::CSSSelector::m_destructorHasBeenCalled): Add bitfield.
+ (WebCore::CSSSelector::CSSSelector): Initialize
+ m_destructorHasBeenCalled.
+ (WebCore::CSSSelector::~CSSSelector): Add security assertion
+ that this is never called twice. Clear out any fields that
+ would have caused us to dereference an object twice.
+
+ * css/CSSSelectorList.cpp:
+ (WebCore::CSSSelectorList::deleteSelectors): Clear
+ m_selectorArray when freeing the memory to which it was
+ pointing. This prevents re-entrancy issues or calling this
+ method twice on the same thread. Also restructure the for()
+ loop to prevent calling CSSSelector::isLastInSelectorList()
+ after CSSSelector::~CSSSelector() has been called (via CRBug
+ 241892).
+
2015-12-09 Simon Fraser <[email protected]>
Merge r191590. rdar://problem/23432368
Modified: branches/safari-601.1.46-branch/Source/WebCore/css/CSSSelector.cpp (193914 => 193915)
--- branches/safari-601.1.46-branch/Source/WebCore/css/CSSSelector.cpp 2015-12-10 18:54:58 UTC (rev 193914)
+++ branches/safari-601.1.46-branch/Source/WebCore/css/CSSSelector.cpp 2015-12-10 19:18:35 UTC (rev 193915)
@@ -64,6 +64,9 @@
, m_descendantDoubleChildSyntax(false)
#endif
, m_caseInsensitiveAttributeValueMatching(false)
+#if !ASSERT_WITH_SECURITY_IMPLICATION_DISABLED
+ , m_destructorHasBeenCalled(false)
+#endif
{
const AtomicString& tagLocalName = tagQName.localName();
const AtomicString tagLocalNameASCIILowercase = tagLocalName.convertToASCIILowercase();
Modified: branches/safari-601.1.46-branch/Source/WebCore/css/CSSSelector.h (193914 => 193915)
--- branches/safari-601.1.46-branch/Source/WebCore/css/CSSSelector.h 2015-12-10 18:54:58 UTC (rev 193914)
+++ branches/safari-601.1.46-branch/Source/WebCore/css/CSSSelector.h 2015-12-10 19:18:35 UTC (rev 193915)
@@ -321,6 +321,9 @@
unsigned m_descendantDoubleChildSyntax : 1;
#endif
unsigned m_caseInsensitiveAttributeValueMatching : 1;
+#if !ASSERT_WITH_SECURITY_IMPLICATION_DISABLED
+ unsigned m_destructorHasBeenCalled : 1;
+#endif
unsigned simpleSelectorSpecificityForPage() const;
@@ -463,6 +466,9 @@
, m_descendantDoubleChildSyntax(false)
#endif
, m_caseInsensitiveAttributeValueMatching(false)
+#if !ASSERT_WITH_SECURITY_IMPLICATION_DISABLED
+ , m_destructorHasBeenCalled(false)
+#endif
{
}
@@ -481,6 +487,9 @@
, m_descendantDoubleChildSyntax(o.m_descendantDoubleChildSyntax)
#endif
, m_caseInsensitiveAttributeValueMatching(o.m_caseInsensitiveAttributeValueMatching)
+#if !ASSERT_WITH_SECURITY_IMPLICATION_DISABLED
+ , m_destructorHasBeenCalled(false)
+#endif
{
if (o.m_hasRareData) {
m_data.m_rareData = o.m_data.m_rareData;
@@ -499,14 +508,26 @@
inline CSSSelector::~CSSSelector()
{
- if (m_hasRareData)
+ ASSERT_WITH_SECURITY_IMPLICATION(!m_destructorHasBeenCalled);
+#if !ASSERT_WITH_SECURITY_IMPLICATION_DISABLED
+ m_destructorHasBeenCalled = true;
+#endif
+ if (m_hasRareData) {
m_data.m_rareData->deref();
- else if (m_hasNameWithCase)
+ m_data.m_rareData = nullptr;
+ m_hasRareData = false;
+ } else if (m_hasNameWithCase) {
m_data.m_nameWithCase->deref();
- else if (match() == Tag)
+ m_data.m_nameWithCase = nullptr;
+ m_hasNameWithCase = false;
+ } else if (match() == Tag) {
m_data.m_tagQName->deref();
- else if (m_data.m_value)
+ m_data.m_tagQName = nullptr;
+ m_match = Unknown;
+ } else if (m_data.m_value) {
m_data.m_value->deref();
+ m_data.m_value = nullptr;
+ }
}
inline const QualifiedName& CSSSelector::tagQName() const
Modified: branches/safari-601.1.46-branch/Source/WebCore/css/CSSSelectorList.cpp (193914 => 193915)
--- branches/safari-601.1.46-branch/Source/WebCore/css/CSSSelectorList.cpp 2015-12-10 18:54:58 UTC (rev 193914)
+++ branches/safari-601.1.46-branch/Source/WebCore/css/CSSSelectorList.cpp 2015-12-10 19:18:35 UTC (rev 193915)
@@ -111,12 +111,15 @@
if (!m_selectorArray)
return;
- for (CSSSelector* s = m_selectorArray; ; ++s) {
+ CSSSelector* selectorArray = m_selectorArray;
+ m_selectorArray = nullptr;
+
+ bool isLastSelector = false;
+ for (CSSSelector* s = selectorArray; !isLastSelector; ++s) {
+ isLastSelector = s->isLastInSelectorList();
s->~CSSSelector();
- if (s->isLastInSelectorList())
- break;
}
- fastFree(m_selectorArray);
+ fastFree(selectorArray);
}
String CSSSelectorList::selectorsText() const
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
