Null pointer crash in DocumentOrderedMap::getElementById via FormAssociatedElement...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Dec 2018 23:06:49 +0000 (23:06 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Dec 2018 23:06:49 +0000 (23:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192392

Reviewed by Dean Jackson.

Source/WebCore:

The crash was caused by FormAssociatedElement::findAssociatedForm invoking DocumentOrderedMap::getElementById
and de-referencing nullptr Attribute* via IdTargetObserver before Element::attributeChanged had updated
ElementData::m_idForStyleResolution.

Fixed it by updating m_idForStyleResolution before invoking IdTargetObservers.

Test: fast/dom/remove-id-form-associated-elemet-id-observer-crash.html

* dom/Element.cpp:
(WebCore::Element::attributeChanged): Fixed the bug.

LayoutTests:

Added a regression test.

* fast/dom/remove-id-form-associated-elemet-id-observer-crash-expected.txt: Added.
* fast/dom/remove-id-form-associated-elemet-id-observer-crash.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238912 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/dom/remove-id-form-associated-elemet-id-observer-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/remove-id-form-associated-elemet-id-observer-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Element.cpp

index c92a071..4a29e50 100644 (file)
@@ -1,3 +1,15 @@
+2018-12-05  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Null pointer crash in DocumentOrderedMap::getElementById via FormAssociatedElement::findAssociatedForm
+        https://bugs.webkit.org/show_bug.cgi?id=192392
+
+        Reviewed by Dean Jackson.
+
+        Added a regression test.
+
+        * fast/dom/remove-id-form-associated-elemet-id-observer-crash-expected.txt: Added.
+        * fast/dom/remove-id-form-associated-elemet-id-observer-crash.html: Added.
+
 2018-12-05  Youenn Fablet  <youenn@apple.com>
 
         [iOS] Layout Test imported/w3c/web-platform-tests/service-workers/service-worker/fetch-cors-xhr.https.html is a flaky failure
diff --git a/LayoutTests/fast/dom/remove-id-form-associated-elemet-id-observer-crash-expected.txt b/LayoutTests/fast/dom/remove-id-form-associated-elemet-id-observer-crash-expected.txt
new file mode 100644 (file)
index 0000000..df78326
--- /dev/null
@@ -0,0 +1,3 @@
+This tests removing the id from an element when there is a form associated element observing the same ID. WebKit should not crash.
+
+PASS
diff --git a/LayoutTests/fast/dom/remove-id-form-associated-elemet-id-observer-crash.html b/LayoutTests/fast/dom/remove-id-form-associated-elemet-id-observer-crash.html
new file mode 100644 (file)
index 0000000..8acde2f
--- /dev/null
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This tests removing the id from an element when there is a form associated element observing the same ID.
+WebKit should not crash.</p>
+<div id="container">
+<p id="foo"></p>
+<form id="foo"></form>
+<fieldset form="foo"></fieldset>
+</div>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+document.querySelector('#container p').removeAttribute('id');
+container.remove();
+document.write('PASS');
+</script>
+</body>
+</html>
index 8eecac1..09db9d3 100644 (file)
@@ -1,3 +1,21 @@
+2018-12-05  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Null pointer crash in DocumentOrderedMap::getElementById via FormAssociatedElement::findAssociatedForm
+        https://bugs.webkit.org/show_bug.cgi?id=192392
+
+        Reviewed by Dean Jackson.
+
+        The crash was caused by FormAssociatedElement::findAssociatedForm invoking DocumentOrderedMap::getElementById
+        and de-referencing nullptr Attribute* via IdTargetObserver before Element::attributeChanged had updated
+        ElementData::m_idForStyleResolution.
+
+        Fixed it by updating m_idForStyleResolution before invoking IdTargetObservers.
+
+        Test: fast/dom/remove-id-form-associated-elemet-id-observer-crash.html
+
+        * dom/Element.cpp:
+        (WebCore::Element::attributeChanged): Fixed the bug.
+
 2018-12-05  Youenn Fablet  <youenn@apple.com>
 
         Enable the possibility to do video capture in UIProcess
index 5a15a69..efd1cf1 100644 (file)
@@ -1500,17 +1500,17 @@ void Element::attributeChanged(const QualifiedName& name, const AtomicString& ol
 
     if (!valueIsSameAsBefore) {
         if (name == HTMLNames::idAttr) {
-            if (!oldValue.isEmpty())
-                treeScope().idTargetObserverRegistry().notifyObservers(*oldValue.impl());
-            if (!newValue.isEmpty())
-                treeScope().idTargetObserverRegistry().notifyObservers(*newValue.impl());
-
             AtomicString oldId = elementData()->idForStyleResolution();
             AtomicString newId = makeIdForStyleResolution(newValue, document().inQuirksMode());
             if (newId != oldId) {
                 Style::IdChangeInvalidation styleInvalidation(*this, oldId, newId);
                 elementData()->setIdForStyleResolution(newId);
             }
+
+            if (!oldValue.isEmpty())
+                treeScope().idTargetObserverRegistry().notifyObservers(*oldValue.impl());
+            if (!newValue.isEmpty())
+                treeScope().idTargetObserverRegistry().notifyObservers(*newValue.impl());
         } else if (name == classAttr)
             classAttributeChanged(newValue);
         else if (name == HTMLNames::nameAttr)