Diff
Modified: branches/safari-537.75-branch/Source/WebCore/ChangeLog (165230 => 165231)
--- branches/safari-537.75-branch/Source/WebCore/ChangeLog 2014-03-07 00:47:18 UTC (rev 165230)
+++ branches/safari-537.75-branch/Source/WebCore/ChangeLog 2014-03-07 00:49:52 UTC (rev 165231)
@@ -22,6 +22,47 @@
2014-03-06 Matthew Hanson <[email protected]>
+ Merge r159489.
+
+ 2013-11-19 Ryosuke Niwa <[email protected]>
+
+ Add more assertions with security implications in DocumentOrderedMap
+ https://bugs.webkit.org/show_bug.cgi?id=124559
+
+ Reviewed by Antti Koivisto.
+
+ Assert that newly added elements and existing elements in the document ordered map are in the same tree scope
+ as the document ordered map. Also exit early if we're about to add an element in a wrong document to the map.
+ We don't exit early in get() because the damage has already been done at that point (the element may have been
+ deleted already).
+
+ * dom/Document.cpp:
+ (WebCore::Document::addImageElementByLowercasedUsemap):
+ * dom/DocumentOrderedMap.cpp:
+ (WebCore::DocumentOrderedMap::add): Assert that the newly added element is in the current tree scope.
+ Also exit early if either the element is not in the tree scope or not in the right document.
+ While this doesn't make the function completely fault safe, it'll catch when we try to add a detached node.
+ (WebCore::DocumentOrderedMap::remove): Convert existing assertions to ones with security implication.
+ (WebCore::DocumentOrderedMap::get): Assert with security implication that the element we're about to return
+ is in the current tree scope. The element may have already been deleted if we ever hit these assertions.
+ (WebCore::DocumentOrderedMap::getAllElementsById): Convert an existing assertion to an assertion with security
+ implication.
+ * dom/DocumentOrderedMap.h:
+ * dom/TreeScope.cpp:
+ (WebCore::TreeScope::addElementById):
+ (WebCore::TreeScope::addElementByName):
+ (WebCore::TreeScope::addImageMap):
+ (WebCore::TreeScope::addLabel):
+ * html/HTMLDocument.cpp:
+ (WebCore::HTMLDocument::addDocumentNamedItem):
+ (WebCore::HTMLDocument::addWindowNamedItem):
+ * html/HTMLImageElement.cpp:
+ (WebCore::HTMLImageElement::insertedInto): Set InTreeScope flag before calling addImageElementByLowercasedUsemap.
+ * html/HTMLMapElement.cpp:
+ (WebCore::HTMLMapElement::insertedInto): Ditto for addImageMap.
+
+2014-03-06 Matthew Hanson <[email protected]>
+
Merge r165145.
2014-03-05 Daniel Bates <[email protected]>
Modified: branches/safari-537.75-branch/Source/WebCore/dom/DocumentOrderedMap.cpp (165230 => 165231)
--- branches/safari-537.75-branch/Source/WebCore/dom/DocumentOrderedMap.cpp 2014-03-07 00:47:18 UTC (rev 165230)
+++ branches/safari-537.75-branch/Source/WebCore/dom/DocumentOrderedMap.cpp 2014-03-07 00:49:52 UTC (rev 165231)
@@ -82,17 +82,18 @@
m_map.clear();
}
-void DocumentOrderedMap::add(AtomicStringImpl* key, Element* element)
+void DocumentOrderedMap::add(AtomicStringImpl* key, Element* element, const TreeScope* treeScope)
{
- ASSERT(key);
- ASSERT(element);
-
+ ASSERT_WITH_SECURITY_IMPLICATION(element->isInTreeScope());
+ ASSERT_WITH_SECURITY_IMPLICATION(treeScope->rootNode()->containsIncludingShadowDOM(element));
+ if (!element->isInTreeScope() || element->document() != treeScope->documentScope())
+ return;
Map::AddResult addResult = m_map.add(key, MapEntry(element));
if (addResult.isNewEntry)
return;
MapEntry& entry = addResult.iterator->value;
- ASSERT(entry.count);
+ ASSERT_WITH_SECURITY_IMPLICATION(entry.count);
entry.element = 0;
entry.count++;
entry.orderedList.clear();
@@ -105,15 +106,14 @@
m_map.checkConsistency();
Map::iterator it = m_map.find(key);
- ASSERT(it != m_map.end());
+ ASSERT_WITH_SECURITY_IMPLICATION(it != m_map.end());
if (it == m_map.end())
return;
-
MapEntry& entry = it->value;
- ASSERT(entry.count);
+ ASSERT_WITH_SECURITY_IMPLICATION(entry.count);
if (entry.count == 1) {
- ASSERT(!entry.element || entry.element == element);
+ ASSERT_WITH_SECURITY_IMPLICATION(!entry.element || entry.element == element);
m_map.remove(it);
} else {
if (entry.element == element)
@@ -137,14 +137,19 @@
MapEntry& entry = it->value;
ASSERT(entry.count);
- if (entry.element)
+ if (entry.element) {
+ ASSERT_WITH_SECURITY_IMPLICATION(entry.element->isInTreeScope());
+ ASSERT_WITH_SECURITY_IMPLICATION(entry.element->treeScope() == scope);
return entry.element;
+ }
// We know there's at least one node that matches; iterate to find the first one.
for (Element* element = ElementTraversal::firstWithin(scope->rootNode()); element; element = ElementTraversal::next(element)) {
if (!keyMatches(key, element))
continue;
entry.element = element;
+ ASSERT_WITH_SECURITY_IMPLICATION(element->isInTreeScope());
+ ASSERT_WITH_SECURITY_IMPLICATION(element->treeScope() == scope);
return element;
}
ASSERT_NOT_REACHED();
@@ -198,7 +203,7 @@
return 0;
MapEntry& entry = it->value;
- ASSERT(entry.count);
+ ASSERT_WITH_SECURITY_IMPLICATION(entry.count);
if (!entry.count)
return 0;
Modified: branches/safari-537.75-branch/Source/WebCore/dom/DocumentOrderedMap.h (165230 => 165231)
--- branches/safari-537.75-branch/Source/WebCore/dom/DocumentOrderedMap.h 2014-03-07 00:47:18 UTC (rev 165230)
+++ branches/safari-537.75-branch/Source/WebCore/dom/DocumentOrderedMap.h 2014-03-07 00:49:52 UTC (rev 165231)
@@ -43,7 +43,7 @@
class DocumentOrderedMap {
public:
- void add(AtomicStringImpl*, Element*);
+ void add(AtomicStringImpl*, Element*, const TreeScope*);
void remove(AtomicStringImpl*, Element*);
void clear();
Modified: branches/safari-537.75-branch/Source/WebCore/dom/Element.cpp (165230 => 165231)
--- branches/safari-537.75-branch/Source/WebCore/dom/Element.cpp 2014-03-07 00:47:18 UTC (rev 165230)
+++ branches/safari-537.75-branch/Source/WebCore/dom/Element.cpp 2014-03-07 00:49:52 UTC (rev 165231)
@@ -2895,7 +2895,7 @@
if (!oldName.isEmpty() && oldName != id)
document->windowNamedItemMap().remove(oldName.impl(), this);
if (!newName.isEmpty() && newName != id)
- document->windowNamedItemMap().add(newName.impl(), this);
+ document->windowNamedItemMap().add(newName.impl(), this, treeScope());
}
if (DocumentNameCollection::nodeMatchesIfNameAttributeMatch(this)) {
@@ -2903,7 +2903,7 @@
if (!oldName.isEmpty() && oldName != id)
document->documentNamedItemMap().remove(oldName.impl(), this);
if (!newName.isEmpty() && newName != id)
- document->documentNamedItemMap().add(newName.impl(), this);
+ document->documentNamedItemMap().add(newName.impl(), this, treeScope());
}
}
@@ -2946,7 +2946,7 @@
if (!oldId.isEmpty() && oldId != name)
document->windowNamedItemMap().remove(oldId.impl(), this);
if (!newId.isEmpty() && newId != name)
- document->windowNamedItemMap().add(newId.impl(), this);
+ document->windowNamedItemMap().add(newId.impl(), this, treeScope());
}
if (DocumentNameCollection::nodeMatchesIfIdAttributeMatch(this)) {
@@ -2954,7 +2954,7 @@
if (!oldId.isEmpty() && oldId != name)
document->documentNamedItemMap().remove(oldId.impl(), this);
if (!newId.isEmpty() && newId != name)
- document->documentNamedItemMap().add(newId.impl(), this);
+ document->documentNamedItemMap().add(newId.impl(), this, treeScope());
}
}
Modified: branches/safari-537.75-branch/Source/WebCore/dom/TreeScope.cpp (165230 => 165231)
--- branches/safari-537.75-branch/Source/WebCore/dom/TreeScope.cpp 2014-03-07 00:47:18 UTC (rev 165230)
+++ branches/safari-537.75-branch/Source/WebCore/dom/TreeScope.cpp 2014-03-07 00:49:52 UTC (rev 165231)
@@ -160,7 +160,7 @@
{
if (!m_elementsById)
m_elementsById = adoptPtr(new DocumentOrderedMap);
- m_elementsById->add(elementId.impl(), element);
+ m_elementsById->add(elementId.impl(), element, this);
m_idTargetObserverRegistry->notifyObservers(elementId);
}
@@ -185,7 +185,7 @@
{
if (!m_elementsByName)
m_elementsByName = adoptPtr(new DocumentOrderedMap);
- m_elementsByName->add(name.impl(), element);
+ m_elementsByName->add(name.impl(), element, this);
}
void TreeScope::removeElementByName(const AtomicString& name, Element* element)
@@ -216,7 +216,7 @@
return;
if (!m_imageMapsByName)
m_imageMapsByName = adoptPtr(new DocumentOrderedMap);
- m_imageMapsByName->add(name, imageMap);
+ m_imageMapsByName->add(name, imageMap, this);
}
void TreeScope::removeImageMap(HTMLMapElement* imageMap)
@@ -281,7 +281,7 @@
void TreeScope::addLabel(const AtomicString& forAttributeValue, HTMLLabelElement* element)
{
ASSERT(m_labelsByForAttribute);
- m_labelsByForAttribute->add(forAttributeValue.impl(), element);
+ m_labelsByForAttribute->add(forAttributeValue.impl(), element, this);
}
void TreeScope::removeLabel(const AtomicString& forAttributeValue, HTMLLabelElement* element)
Modified: branches/safari-537.75-branch/Source/WebCore/html/HTMLImageElement.cpp (165230 => 165231)
--- branches/safari-537.75-branch/Source/WebCore/html/HTMLImageElement.cpp 2014-03-07 00:47:18 UTC (rev 165230)
+++ branches/safari-537.75-branch/Source/WebCore/html/HTMLImageElement.cpp 2014-03-07 00:49:52 UTC (rev 165231)
@@ -132,7 +132,7 @@
const AtomicString& id = getIdAttribute();
if (!id.isEmpty() && id != getNameAttribute()) {
if (willHaveName)
- document->documentNamedItemMap().add(id.impl(), this);
+ document->documentNamedItemMap().add(id.impl(), this, treeScope());
else
document->documentNamedItemMap().remove(id.impl(), this);
}
Modified: branches/safari-537.75-branch/Source/WebCore/html/HTMLMapElement.cpp (165230 => 165231)
--- branches/safari-537.75-branch/Source/WebCore/html/HTMLMapElement.cpp 2014-03-07 00:47:18 UTC (rev 165230)
+++ branches/safari-537.75-branch/Source/WebCore/html/HTMLMapElement.cpp 2014-03-07 00:49:52 UTC (rev 165231)
@@ -133,9 +133,10 @@
Node::InsertionNotificationRequest HTMLMapElement::insertedInto(ContainerNode* insertionPoint)
{
+ Node::InsertionNotificationRequest request = HTMLElement::insertedInto(insertionPoint);
if (insertionPoint->inDocument())
treeScope()->addImageMap(this);
- return HTMLElement::insertedInto(insertionPoint);
+ return request;
}
void HTMLMapElement::removedFrom(ContainerNode* insertionPoint)
Modified: branches/safari-537.75-branch/Source/WebCore/html/HTMLObjectElement.cpp (165230 => 165231)
--- branches/safari-537.75-branch/Source/WebCore/html/HTMLObjectElement.cpp 2014-03-07 00:47:18 UTC (rev 165230)
+++ branches/safari-537.75-branch/Source/WebCore/html/HTMLObjectElement.cpp 2014-03-07 00:49:52 UTC (rev 165231)
@@ -445,7 +445,7 @@
const AtomicString& id = getIdAttribute();
if (!id.isEmpty()) {
if (isNamedItem)
- document->documentNamedItemMap().add(id.impl(), this);
+ document->documentNamedItemMap().add(id.impl(), this, treeScope());
else
document->documentNamedItemMap().remove(id.impl(), this);
}
@@ -453,7 +453,7 @@
const AtomicString& name = getNameAttribute();
if (!name.isEmpty() && id != name) {
if (isNamedItem)
- document->documentNamedItemMap().add(name.impl(), this);
+ document->documentNamedItemMap().add(name.impl(), this, treeScope());
else
document->documentNamedItemMap().remove(name.impl(), this);
}
