CSP blocks inline style when cloning a node
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 3 Apr 2013 15:45:10 +0000 (15:45 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 3 Apr 2013 15:45:10 +0000 (15:45 +0000)
https://bugs.webkit.org/show_bug.cgi?id=112270

Patch by Felipe Zimmerle <felipe@zimmerle.org> on 2013-04-03
Reviewed by Adam Barth.

Source/WebCore:

Checks if the Element is being cloned, if so, the application of the
style is allowed otherwise it relies on default permission mechanism.

Test: http/tests/security/contentSecurityPolicy/inline-style-allowed-while-cloning-objects.html

* dom/Element.cpp:
(WebCore::Element::attributeChanged): Added parameter
AttributeModificationReason to the method signature.
(WebCore::Element::cloneAttributesFromElement): It is now calling
attributeChanged with AttributeModificationReason parameter. In this scope
it is always set to ModifiedByCloning.
(WebCode::Element::attributeChangedFromParserOrByCloning):
Added parameter AttributeModificationReason.
* dom/Element.h: Added AttributeModificationReason enum. Used to specify
whenever an attribute was set by a cloned oject or directly.
AttributeModificationReason added to attributeChanged with the default
value set to ModifiedDirectly.
* dom/StyledElement.cpp:
(WebCore::StyledElement::attributeChanged): Added
AttributeModificationReason to the method signature.
(WebCore::StyledElement::styleAttributeChanged): Now it is checking the
reason of the update, if the reason is ModifiedDirectly check CSP
before set. If ModifiedByCloning set the attribute ignoring the CSP
policy.
* dom/StyledElement.h:
(StyledElement): Added AttributeModificationReason parameter to the methods:
attributeChanged and styleAttributeChanged, attributeChanged has
ModifiedDirectly as default value.
* svg/SVGElement.cpp:
(WebCore::SVGElement::attributeChanged): Added AttributeModificationReason
parameter to the method signature.
* svg/SVGElement.h:
(SVGElement): Added AttributeModificationReason parameter to the
attributeChanged signature with the default value set to:
ModifiedDirectly.

LayoutTests:

* http/tests/security/contentSecurityPolicy/inline-style-allowed-while-cloning-objects-expected.txt: Added.
* http/tests/security/contentSecurityPolicy/inline-style-allowed-while-cloning-objects.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/http/tests/security/contentSecurityPolicy/inline-style-allowed-while-cloning-objects-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/contentSecurityPolicy/inline-style-allowed-while-cloning-objects.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Element.cpp
Source/WebCore/dom/Element.h
Source/WebCore/dom/StyledElement.cpp
Source/WebCore/dom/StyledElement.h
Source/WebCore/svg/SVGElement.cpp
Source/WebCore/svg/SVGElement.h

index 0ca83b8..28d9ca2 100644 (file)
@@ -1,3 +1,13 @@
+2013-04-03  Felipe Zimmerle  <felipe@zimmerle.org>
+
+        CSP blocks inline style when cloning a node
+        https://bugs.webkit.org/show_bug.cgi?id=112270
+
+        Reviewed by Adam Barth.
+
+        * http/tests/security/contentSecurityPolicy/inline-style-allowed-while-cloning-objects-expected.txt: Added.
+        * http/tests/security/contentSecurityPolicy/inline-style-allowed-while-cloning-objects.html: Added.
+
 2013-04-03  Ádám Kallai  <kadam@inf.u-szeged.hu>
 
         [Qt] Unreviewed gardening. Updated platform specific expected files after r147492.
diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/inline-style-allowed-while-cloning-objects-expected.txt b/LayoutTests/http/tests/security/contentSecurityPolicy/inline-style-allowed-while-cloning-objects-expected.txt
new file mode 100644 (file)
index 0000000..3f91bc1
--- /dev/null
@@ -0,0 +1,38 @@
+CONSOLE MESSAGE: Refused to apply inline style because it violates the following Content Security Policy directive: "style-src 'self'".
+
+CONSOLE MESSAGE: line 79: Refused to apply inline style because it violates the following Content Security Policy directive: "style-src 'self'".
+
+This test ensures that styles can be set by object.cloneNode()
+PASS node1.style.background is "yellow"
+PASS node2.style.background is "yellow"
+PASS node3.style.background is "blue"
+PASS node4.style.background is "blue"
+PASS node1.style.color is "red"
+PASS node2.style.color is "red"
+PASS node3.style.color is "green"
+PASS node4.style.color is "green"
+PASS window.getComputedStyle(node1).getPropertyCSSValue('background').cssText is window.getComputedStyle(node2).getPropertyCSSValue('background').cssText
+PASS window.getComputedStyle(node3).getPropertyCSSValue('background').cssText is window.getComputedStyle(node4).getPropertyCSSValue('background').cssText
+PASS window.getComputedStyle(node1).getPropertyCSSValue('color').cssText is window.getComputedStyle(node2).getPropertyCSSValue('color').cssText
+PASS window.getComputedStyle(node3).getPropertyCSSValue('color').cssText is window.getComputedStyle(node4).getPropertyCSSValue('color').cssText
+PASS ops.style.background is ""
+getComputedStyle(clonedOps).getPropertyCSSValue('background').cssText: rgba(0, 0, 0, 0) none repeat scroll 0% 0% / auto padding-box border-box
+PASS ops.style.color is "red"
+PASS clonedOps.style.background is ""
+PASS violetOps.style.background is "rgb(238, 130, 238)"
+PASS window.getComputedStyle(clonedOps).getPropertyCSSValue('background').cssText is window.getComputedStyle(ops).getPropertyCSSValue('background').cssText
+PASS window.getComputedStyle(clonedOps).getPropertyCSSValue('color').cssText is window.getComputedStyle(ops).getPropertyCSSValue('color').cssText
+getComputedStyle(violetOps).getPropertyCSSValue('background').cssText: rgb(238, 130, 238) none repeat scroll 0% 0% / auto padding-box border-box
+PASS window.getComputedStyle(ops).getPropertyCSSValue('background').cssText is not window.getComputedStyle(violetOps).getPropertyCSSValue('background').cssText
+PASS window.getComputedStyle(clonedOps).getPropertyCSSValue('background').cssText is not window.getComputedStyle(violetOps).getPropertyCSSValue('background').cssText
+PASS ops.id is "ops"
+PASS ops.id is clonedOps.id
+Bug 112270 has more information on the subject.
+This is a div (nodes)
+This is a div. (node 1 or 2)
+This is a div. (node 1 or 2)
+This is a div. (node 3 or 4)
+Node #4
+Yet another div.
+Yet another div.
+Yet another div.
diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/inline-style-allowed-while-cloning-objects.html b/LayoutTests/http/tests/security/contentSecurityPolicy/inline-style-allowed-while-cloning-objects.html
new file mode 100644 (file)
index 0000000..fe25ebb
--- /dev/null
@@ -0,0 +1,87 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <meta http-equiv="Content-Security-Policy" content="style-src 'self';">
+    <script src="../../resources/js-test-pre.js"></script>
+    <script>
+        window.onload = function () {
+            debug("This test ensures that styles can be set by object.cloneNode()");
+
+            window.nodes = document.getElementById('nodes');
+            window.node1 = document.getElementById('node1');
+            window.node1.style.background = "yellow";
+            window.node1.style.color = "red";
+            window.node2 = document.getElementById('node1').cloneNode(true);
+            window.node2.id =  "node2";
+            window.node3 = document.getElementById('node3');
+            window.node3.style.background = "blue";
+            window.node3.style.color = "green";
+            window.node4 = document.getElementById('node3').cloneNode(false);
+            window.node4.id =  "node4";
+            window.node4.innerHTML = "Node #4";
+
+            nodes.appendChild(node1);
+            nodes.appendChild(node2);
+            nodes.appendChild(node3);
+            nodes.appendChild(node4);
+
+            shouldBeEqualToString("node1.style.background", "yellow");
+            shouldBeEqualToString("node2.style.background", "yellow");
+            shouldBeEqualToString("node3.style.background", "blue");
+            shouldBeEqualToString("node4.style.background", "blue");
+  
+            shouldBeEqualToString("node1.style.color", "red");
+            shouldBeEqualToString("node2.style.color", "red");
+            shouldBeEqualToString("node3.style.color", "green");
+            shouldBeEqualToString("node4.style.color", "green");
+
+            shouldBe("window.getComputedStyle(node1).getPropertyCSSValue('background').cssText", "window.getComputedStyle(node2).getPropertyCSSValue('background').cssText");
+            shouldBe("window.getComputedStyle(node3).getPropertyCSSValue('background').cssText", "window.getComputedStyle(node4).getPropertyCSSValue('background').cssText");
+
+            shouldBe("window.getComputedStyle(node1).getPropertyCSSValue('color').cssText", "window.getComputedStyle(node2).getPropertyCSSValue('color').cssText");
+            shouldBe("window.getComputedStyle(node3).getPropertyCSSValue('color').cssText", "window.getComputedStyle(node4).getPropertyCSSValue('color').cssText");
+
+            window.ops = document.getElementById('ops');
+            ops.style.color = 'red';
+            window.clonedOps = ops.cloneNode(true);
+            window.violetOps = document.getElementById('violetOps');
+
+            violetOps.style.background = 'rgb(238, 130, 238)';
+            document.getElementsByTagName('body')[0].appendChild(clonedOps);
+
+            shouldBeEqualToString("ops.style.background", "");
+            debug("getComputedStyle(clonedOps).getPropertyCSSValue('background').cssText: " + window.getComputedStyle(ops).getPropertyCSSValue('background').cssText);
+            shouldBeEqualToString("ops.style.color", "red");
+            shouldBeEqualToString("clonedOps.style.background", "");
+            shouldBeEqualToString("violetOps.style.background", "rgb(238, 130, 238)");
+
+            shouldBe("window.getComputedStyle(clonedOps).getPropertyCSSValue('background').cssText", "window.getComputedStyle(ops).getPropertyCSSValue('background').cssText");
+            shouldBe("window.getComputedStyle(clonedOps).getPropertyCSSValue('color').cssText", "window.getComputedStyle(ops).getPropertyCSSValue('color').cssText");
+            debug("getComputedStyle(violetOps).getPropertyCSSValue('background').cssText: " + window.getComputedStyle(violetOps).getPropertyCSSValue('background').cssText);
+            shouldNotBe("window.getComputedStyle(ops).getPropertyCSSValue('background').cssText", "window.getComputedStyle(violetOps).getPropertyCSSValue('background').cssText");
+            shouldNotBe("window.getComputedStyle(clonedOps).getPropertyCSSValue('background').cssText", "window.getComputedStyle(violetOps).getPropertyCSSValue('background').cssText");
+
+            shouldBeEqualToString("ops.id", "ops");
+            shouldBe("ops.id", "clonedOps.id");
+       };
+    </script>
+</head>
+<body>
+   <a href="https://bugs.webkit.org/show_bug.cgi?id=112270">Bug 112270</a>
+has more information on the subject.</p>
+
+    <div id="nodes">
+       This is a div (nodes)
+        <div id="node1"> This is a div. (node 1 or 2)</div>
+        <div id="node3"> This is a div. (node 3 or 4)</div>
+    </div>
+
+    <div id="ops" style="background: rgb(238, 130, 238)">
+         Yet another div.
+    </div>
+    <div id="violetOps">
+         Yet another div.
+    </div>
+
+</body>
+</html>
index 9c6e8fa..d30216b 100644 (file)
@@ -1,3 +1,46 @@
+2013-04-03  Felipe Zimmerle  <felipe@zimmerle.org>
+
+        CSP blocks inline style when cloning a node
+        https://bugs.webkit.org/show_bug.cgi?id=112270
+
+        Reviewed by Adam Barth.
+
+        Checks if the Element is being cloned, if so, the application of the
+        style is allowed otherwise it relies on default permission mechanism.
+
+        Test: http/tests/security/contentSecurityPolicy/inline-style-allowed-while-cloning-objects.html
+
+        * dom/Element.cpp:
+        (WebCore::Element::attributeChanged): Added parameter
+        AttributeModificationReason to the method signature.
+        (WebCore::Element::cloneAttributesFromElement): It is now calling 
+        attributeChanged with AttributeModificationReason parameter. In this scope
+        it is always set to ModifiedByCloning.
+        (WebCode::Element::attributeChangedFromParserOrByCloning):
+        Added parameter AttributeModificationReason.
+        * dom/Element.h: Added AttributeModificationReason enum. Used to specify
+        whenever an attribute was set by a cloned oject or directly.
+        AttributeModificationReason added to attributeChanged with the default
+        value set to ModifiedDirectly. 
+        * dom/StyledElement.cpp: 
+        (WebCore::StyledElement::attributeChanged): Added
+        AttributeModificationReason to the method signature.
+        (WebCore::StyledElement::styleAttributeChanged): Now it is checking the
+        reason of the update, if the reason is ModifiedDirectly check CSP
+        before set. If ModifiedByCloning set the attribute ignoring the CSP
+        policy.
+        * dom/StyledElement.h:
+        (StyledElement): Added AttributeModificationReason parameter to the methods:
+        attributeChanged and styleAttributeChanged, attributeChanged has
+        ModifiedDirectly as default value.
+        * svg/SVGElement.cpp: 
+        (WebCore::SVGElement::attributeChanged): Added AttributeModificationReason
+        parameter to the method signature.
+        * svg/SVGElement.h: 
+        (SVGElement): Added AttributeModificationReason parameter to the
+        attributeChanged signature with the default value set to:
+        ModifiedDirectly.
+
 2013-04-03  Csaba Osztrogonác  <ossy@webkit.org>
 
         Unreviewed 32bit buildfix after r147542.
index f620f99..b895402 100644 (file)
@@ -847,7 +847,7 @@ static bool checkNeedsStyleInvalidationForIdChange(const AtomicString& oldId, co
     return false;
 }
 
-void Element::attributeChanged(const QualifiedName& name, const AtomicString& newValue)
+void Element::attributeChanged(const QualifiedName& name, const AtomicString& newValue, AttributeModificationReason)
 {
     if (ElementShadow* parentElementShadow = shadowOfParentForDistribution(this)) {
         if (shouldInvalidateDistributionWhenAttributeChanged(parentElementShadow, name, newValue))
@@ -890,7 +890,7 @@ void Element::attributeChanged(const QualifiedName& name, const AtomicString& ne
         cache->handleAttributeChanged(name, this);
 }
 
-inline void Element::attributeChangedFromParserOrByCloning(const QualifiedName& name, const AtomicString& newValue)
+inline void Element::attributeChangedFromParserOrByCloning(const QualifiedName& name, const AtomicString& newValue, AttributeModificationReason reason)
 {
 #if ENABLE(CUSTOM_ELEMENTS)
     if (name == isAttr) {
@@ -898,7 +898,7 @@ inline void Element::attributeChangedFromParserOrByCloning(const QualifiedName&
             registry->didGiveTypeExtension(this);
     }
 #endif
-    attributeChanged(name, newValue);
+    attributeChanged(name, newValue, reason);
 }
 
 template <typename CharacterType>
@@ -1076,7 +1076,7 @@ void Element::parserSetAttributes(const Vector<Attribute>& attributeVector)
 
     // Use attributeVector instead of m_elementData because attributeChanged might modify m_elementData.
     for (unsigned i = 0; i < attributeVector.size(); ++i)
-        attributeChangedFromParserOrByCloning(attributeVector[i].name(), attributeVector[i].value());
+        attributeChangedFromParserOrByCloning(attributeVector[i].name(), attributeVector[i].value(), ModifiedDirectly);
 }
 
 bool Element::hasAttributes() const
@@ -2891,7 +2891,7 @@ void Element::cloneAttributesFromElement(const Element& other)
 
     for (unsigned i = 0; i < m_elementData->length(); ++i) {
         const Attribute* attribute = const_cast<const ElementData*>(m_elementData.get())->attributeItem(i);
-        attributeChangedFromParserOrByCloning(attribute->name(), attribute->value());
+        attributeChangedFromParserOrByCloning(attribute->name(), attribute->value(), ModifiedByCloning);
     }
 }
 
index d2bb5db..514f580 100644 (file)
@@ -373,8 +373,13 @@ public:
     // For exposing to DOM only.
     NamedNodeMap* attributes() const;
 
+    enum AttributeModificationReason {
+        ModifiedDirectly,
+        ModifiedByCloning
+    };
+
     // This method is called whenever an attribute is added, changed or removed.
-    virtual void attributeChanged(const QualifiedName&, const AtomicString&);
+    virtual void attributeChanged(const QualifiedName&, const AtomicString&, AttributeModificationReason = ModifiedDirectly);
     virtual void parseAttribute(const QualifiedName&, const AtomicString&) { }
 
     // Only called by the parser immediately after element construction.
@@ -686,7 +691,7 @@ private:
     void setAttributeInternal(size_t index, const QualifiedName&, const AtomicString& value, SynchronizationOfLazyAttribute);
     void addAttributeInternal(const QualifiedName&, const AtomicString& value, SynchronizationOfLazyAttribute);
     void removeAttributeInternal(size_t index, SynchronizationOfLazyAttribute);
-    void attributeChangedFromParserOrByCloning(const QualifiedName&, const AtomicString&);
+    void attributeChangedFromParserOrByCloning(const QualifiedName&, const AtomicString&, AttributeModificationReason);
 
 #ifndef NDEBUG
     virtual void formatForDebugger(char* buffer, unsigned length) const;
index 9a2c832..93ae33d 100644 (file)
@@ -157,16 +157,16 @@ MutableStylePropertySet* StyledElement::ensureMutableInlineStyle()
     return static_cast<MutableStylePropertySet*>(inlineStyle.get());
 }
 
-void StyledElement::attributeChanged(const QualifiedName& name, const AtomicString& newValue)
+void StyledElement::attributeChanged(const QualifiedName& name, const AtomicString& newValue, AttributeModificationReason reason)
 {
     if (name == styleAttr)
-        styleAttributeChanged(newValue);
+        styleAttributeChanged(newValue, reason);
     else if (isPresentationAttribute(name)) {
         elementData()->m_presentationAttributeStyleIsDirty = true;
         setNeedsStyleRecalc(InlineStyleChange);
     }
 
-    Element::attributeChanged(name, newValue);
+    Element::attributeChanged(name, newValue, reason);
 }
 
 PropertySetCSSStyleDeclaration* StyledElement::inlineStyleCSSOMWrapper()
@@ -197,7 +197,7 @@ inline void StyledElement::setInlineStyleFromString(const AtomicString& newStyle
         inlineStyle->parseDeclaration(newStyleString, document()->elementSheet()->contents());
 }
 
-void StyledElement::styleAttributeChanged(const AtomicString& newStyleString)
+void StyledElement::styleAttributeChanged(const AtomicString& newStyleString, AttributeModificationReason reason)
 {
     WTF::OrdinalNumber startLineNumber = WTF::OrdinalNumber::beforeFirst();
     if (document() && document()->scriptableDocumentParser() && !document()->isInDocumentWrite())
@@ -207,7 +207,7 @@ void StyledElement::styleAttributeChanged(const AtomicString& newStyleString)
         if (PropertySetCSSStyleDeclaration* cssomWrapper = inlineStyleCSSOMWrapper())
             cssomWrapper->clearParentElement();
         ensureUniqueElementData()->m_inlineStyle.clear();
-    } else if (document()->contentSecurityPolicy()->allowInlineStyle(document()->url(), startLineNumber))
+    } else if (reason == ModifiedByCloning || document()->contentSecurityPolicy()->allowInlineStyle(document()->url(), startLineNumber))
         setInlineStyleFromString(newStyleString);
 
     elementData()->m_styleAttributeIsDirty = false;
index dd69e8f..3134473 100644 (file)
@@ -66,7 +66,7 @@ protected:
     {
     }
 
-    virtual void attributeChanged(const QualifiedName&, const AtomicString&) OVERRIDE;
+    virtual void attributeChanged(const QualifiedName&, const AtomicString&, AttributeModificationReason = ModifiedDirectly) OVERRIDE;
 
     virtual bool isPresentationAttribute(const QualifiedName&) const { return false; }
 
@@ -77,7 +77,7 @@ protected:
     virtual void addSubresourceAttributeURLs(ListHashSet<KURL>&) const;
 
 private:
-    void styleAttributeChanged(const AtomicString& newStyleString);
+    void styleAttributeChanged(const AtomicString& newStyleString, AttributeModificationReason);
 
     void inlineStyleChanged();
     PropertySetCSSStyleDeclaration* inlineStyleCSSOMWrapper();
index 50ee96a..eaf8bc9 100644 (file)
@@ -530,7 +530,7 @@ bool SVGElement::childShouldCreateRenderer(const NodeRenderingContext& childCont
     return false;
 }
 
-void SVGElement::attributeChanged(const QualifiedName& name, const AtomicString& newValue)
+void SVGElement::attributeChanged(const QualifiedName& name, const AtomicString& newValue, AttributeModificationReason)
 {
     StyledElement::attributeChanged(name, newValue);
 
index bff83a8..db74c29 100644 (file)
@@ -126,7 +126,7 @@ protected:
     virtual void parseAttribute(const QualifiedName&, const AtomicString&) OVERRIDE;
 
     virtual void finishParsingChildren();
-    virtual void attributeChanged(const QualifiedName&, const AtomicString&) OVERRIDE;
+    virtual void attributeChanged(const QualifiedName&, const AtomicString&, AttributeModificationReason = ModifiedDirectly) OVERRIDE;
     virtual bool childShouldCreateRenderer(const NodeRenderingContext&) const OVERRIDE;
     
     virtual void removedFrom(ContainerNode*) OVERRIDE;