Cross-Origin copy&paste / drag&drop allowing XSS via srcdoc attribute.
authortsepez@chromium.org <tsepez@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 30 Mar 2013 18:31:52 +0000 (18:31 +0000)
committertsepez@chromium.org <tsepez@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 30 Mar 2013 18:31:52 +0000 (18:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=113443

Reviewed by Adam Barth.

Source/WebCore:

Tested by LayoutTests/editing/pasteboard/paste-noscript.html

* dom/Element.h:
(Element):
(WebCore::Element::isHTMLContentAttribute):
Adds an isHTMLContentAttribute() method to determine whether an attribute can contain
(potentially unsafe) HTML content. Currently, the iframe's srcdoc attribute is the only
such attribute, but clever folks might add more in the future.
Rename stripJavaScriptAttributes() method to stripScriptingAttributes(), to better reflect
the reality that scripting content may appear as above.
Adds missing consts and consolidate isJavaScriptAttribute() method.

* dom/Element.cpp:
(WebCore::Element::isJavaScriptURLAttribute):
(WebCore::Element::stripScriptingAttributes):
Consolidated methods.

* html/HTMLFrameElementBase.cpp:
(WebCore::HTMLFrameElementBase::isHTMLContentAttribute):
(WebCore):
* html/HTMLFrameElementBase.h:
(HTMLFrameElementBase):
Indicate that for frames, the srcdoc attribute contains HTML content.

* html/parser/HTMLConstructionSite.cpp:
(WebCore::setAttributes):
* xml/parser/XMLDocumentParserLibxml2.cpp:
(WebCore::setAttributes):
* xml/parser/XMLDocumentParserQt.cpp:
(WebCore::setAttributes):
Rename stripJavaScriptAttribute calls to match Element.h

LayoutTests:

* editing/pasteboard/paste-noscript-expected.txt:
* editing/pasteboard/paste-noscript.html:
Adds a test that an iframe's srcdoc attribute is not pasted.

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

LayoutTests/ChangeLog
LayoutTests/editing/pasteboard/paste-noscript-expected.txt
LayoutTests/editing/pasteboard/paste-noscript.html
Source/WebCore/ChangeLog
Source/WebCore/dom/Element.cpp
Source/WebCore/dom/Element.h
Source/WebCore/html/HTMLFrameElementBase.cpp
Source/WebCore/html/HTMLFrameElementBase.h
Source/WebCore/html/parser/HTMLConstructionSite.cpp
Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp
Source/WebCore/xml/parser/XMLDocumentParserQt.cpp

index b4880eb..64707ba 100644 (file)
@@ -1,5 +1,16 @@
 2013-03-30  Tom Sepez  <tsepez@chromium.org>
 
+        Cross-Origin copy&paste / drag&drop allowing XSS via srcdoc attribute.
+        https://bugs.webkit.org/show_bug.cgi?id=113443
+
+        Reviewed by Adam Barth.
+
+        * editing/pasteboard/paste-noscript-expected.txt:
+        * editing/pasteboard/paste-noscript.html:
+        Adds a test that an iframe's srcdoc attribute is not pasted.
+
+2013-03-30  Tom Sepez  <tsepez@chromium.org>
+
         View-source iframes are dangerous (and not very useful).
         https://bugs.webkit.org/show_bug.cgi?id=113345
 
index 9daea30..3c64d47 100644 (file)
@@ -1,10 +1,10 @@
 This test copies all the elements containing event handlers and javascript urls, pastes them in an editable area and verifies that no script, handlers or javascript urls are copied.
 Hello 
-CNN Hello 
+CNN Hello  
 This is a form
 Submit.
 Hello 
-CNN Hello 
+CNN Hello  
 This is a form
 Submit.
 <button id="button1" onclick="sayHello()" ondblclick="sayHello()" style="width: 100px;">Hello</button>
@@ -15,5 +15,7 @@ Submit.
 <a id="anchor2">Hello</a>
 <iframe id="iframe1" src="javascript:var x = 1;" style="width: 200px; height: 100px; background-color:#cee;"></iframe>
 <iframe id="iframe1" style="width: 200px; height: 100px; background-color: rgb(204, 238, 238);"></iframe>
+<iframe id="iframe2" srcdoc="&lt;script&gt;var x = 1;&lt;/script&gt;" style="width: 200px; height: 100px; background-color:#cee;"></iframe>
+<iframe id="iframe2" style="width: 200px; height: 100px; background-color: rgb(204, 238, 238);"></iframe>
 <form id="form1" action="javascript:sayHello()" formaction="javascript:sayHello()" style="width: 200px; height: 150px; background-color:#cee;">This is a form<br><img src="../resources/abe.png"><button formaction="javascript:sayHello()">Submit.</button></form>
 <form id="form1" formaction="javascript:sayHello()" style="width: 200px; height: 150px; background-color: rgb(204, 238, 238);">This is a form<br><img src="../resources/abe.png"><button>Submit.</button></form>
index 234d876..58bdfbb 100644 (file)
@@ -16,6 +16,7 @@ in an editable area and verifies that no script, handlers or javascript urls are
 <a id="anchor1" href="http://www.cnn.com/">CNN</a>
 <a id="anchor2" href="javascript:sayHello()">Hello</a>
 <iframe id="iframe1" src="javascript:var x = 1;" style="width: 200px; height: 100px; background-color:#cee;"></iframe>
+<iframe id="iframe2" srcdoc="<script>var x = 1;</script>" style="width: 200px; height: 100px; background-color:#cee;"></iframe>
 <form id="form1" action="javascript:sayHello()" formaction="javascript:sayHello()" style="width: 200px; height: 150px; background-color:#cee;">This is a form<br><img src="../resources/abe.png"></img><button formaction="javascript:sayHello()">Submit.</button></form>
 </div>
 <div id="pastehere" contenteditable="true">
@@ -25,7 +26,7 @@ in an editable area and verifies that no script, handlers or javascript urls are
 var s = window.getSelection();
 var p1 = document.getElementById("test");
 s.setPosition(p1, 0);
-s.setBaseAndExtent(p1, 0, p1, 12);
+s.setBaseAndExtent(p1, 0, p1, 14);
 document.execCommand("Copy");
 p1 = document.getElementById("pastehere");
 s.setPosition(p1, 0);
@@ -43,8 +44,11 @@ log(document.getElementById("pastehere").childNodes[5].outerHTML);
 log(document.getElementById("iframe1").outerHTML);
 log(document.getElementById("pastehere").childNodes[7].outerHTML);
 
+log(document.getElementById("iframe2").outerHTML);
+log(document.getElementById("pastehere").childNodes[9].outerHTML);
+
 log(document.getElementById("form1").outerHTML);
-log(document.getElementById("pastehere").childNodes[8].outerHTML);
+log(document.getElementById("pastehere").childNodes[10].outerHTML);
 
 function log(str) {
     var li = document.createElement("li");
index 2b194ee..5f723e5 100644 (file)
@@ -1,5 +1,44 @@
 2013-03-30  Tom Sepez  <tsepez@chromium.org>
 
+        Cross-Origin copy&paste / drag&drop allowing XSS via srcdoc attribute.
+        https://bugs.webkit.org/show_bug.cgi?id=113443
+
+        Reviewed by Adam Barth.
+
+        Tested by LayoutTests/editing/pasteboard/paste-noscript.html
+
+        * dom/Element.h:
+        (Element):
+        (WebCore::Element::isHTMLContentAttribute):
+        Adds an isHTMLContentAttribute() method to determine whether an attribute can contain
+        (potentially unsafe) HTML content. Currently, the iframe's srcdoc attribute is the only
+        such attribute, but clever folks might add more in the future.
+        Rename stripJavaScriptAttributes() method to stripScriptingAttributes(), to better reflect
+        the reality that scripting content may appear as above.
+        Adds missing consts and consolidate isJavaScriptAttribute() method.
+
+        * dom/Element.cpp:
+        (WebCore::Element::isJavaScriptURLAttribute):
+        (WebCore::Element::stripScriptingAttributes):
+        Consolidated methods.
+        
+        * html/HTMLFrameElementBase.cpp:
+        (WebCore::HTMLFrameElementBase::isHTMLContentAttribute):
+        (WebCore):
+        * html/HTMLFrameElementBase.h:
+        (HTMLFrameElementBase):
+        Indicate that for frames, the srcdoc attribute contains HTML content.
+        
+        * html/parser/HTMLConstructionSite.cpp:
+        (WebCore::setAttributes):
+        * xml/parser/XMLDocumentParserLibxml2.cpp:
+        (WebCore::setAttributes):
+        * xml/parser/XMLDocumentParserQt.cpp:
+        (WebCore::setAttributes):
+        Rename stripJavaScriptAttribute calls to match Element.h
+
+2013-03-30  Tom Sepez  <tsepez@chromium.org>
+
         View-source iframes are dangerous (and not very useful).
         https://bugs.webkit.org/show_bug.cgi?id=113345
 
index 50dac8b..0387a12 100644 (file)
@@ -1039,29 +1039,18 @@ static inline bool isEventHandlerAttribute(const Attribute& attribute)
     return attribute.name().namespaceURI().isNull() && attribute.name().localName().startsWith("on");
 }
 
-bool Element::isJavaScriptURLAttribute(const Attribute& attribute)
+bool Element::isJavaScriptURLAttribute(const Attribute& attribute) const
 {
-    if (!isURLAttribute(attribute))
-        return false;
-    if (!protocolIsJavaScript(stripLeadingAndTrailingHTMLSpaces(attribute.value())))
-        return false;
-    return true;
-}
-
-bool Element::isJavaScriptAttribute(const Attribute& attribute)
-{
-    if (isEventHandlerAttribute(attribute))
-        return true;
-    if (isJavaScriptURLAttribute(attribute))
-        return true;
-    return false;
+    return isURLAttribute(attribute) && protocolIsJavaScript(stripLeadingAndTrailingHTMLSpaces(attribute.value()));
 }
 
-void Element::stripJavaScriptAttributes(Vector<Attribute>& attributeVector)
+void Element::stripScriptingAttributes(Vector<Attribute>& attributeVector) const
 {
     size_t destination = 0;
     for (size_t source = 0; source < attributeVector.size(); ++source) {
-        if (isJavaScriptAttribute(attributeVector[source]))
+        if (isEventHandlerAttribute(attributeVector[source])
+            || isJavaScriptURLAttribute(attributeVector[source])
+            || isHTMLContentAttribute(attributeVector[source]))
             continue;
 
         if (source != destination)
index 1ec9e30..d2bb5db 100644 (file)
@@ -380,7 +380,8 @@ public:
     // Only called by the parser immediately after element construction.
     void parserSetAttributes(const Vector<Attribute>&);
 
-    void stripJavaScriptAttributes(Vector<Attribute>&);
+    // Remove attributes that might introduce scripting from the vector leaving the element unchanged.
+    void stripScriptingAttributes(Vector<Attribute>&) const;
 
     const ElementData* elementData() const { return m_elementData.get(); }
     UniqueElementData* ensureUniqueElementData();
@@ -454,6 +455,7 @@ public:
     virtual void accessKeyAction(bool /*sendToAnyEvent*/) { }
 
     virtual bool isURLAttribute(const Attribute&) const { return false; }
+    virtual bool isHTMLContentAttribute(const Attribute&) const { return false; }
 
     KURL getURLAttribute(const QualifiedName&) const;
     KURL getNonEmptyURLAttribute(const QualifiedName&) const;
@@ -735,8 +737,7 @@ private:
 
     void createRendererIfNeeded();
 
-    bool isJavaScriptAttribute(const Attribute&);
-    bool isJavaScriptURLAttribute(const Attribute&);
+    bool isJavaScriptURLAttribute(const Attribute&) const;
 
     RefPtr<ElementData> m_elementData;
 };
index 7bbc4aa..b7accea 100644 (file)
@@ -222,6 +222,11 @@ bool HTMLFrameElementBase::isURLAttribute(const Attribute& attribute) const
     return attribute.name() == srcAttr || HTMLFrameOwnerElement::isURLAttribute(attribute);
 }
 
+bool HTMLFrameElementBase::isHTMLContentAttribute(const Attribute& attribute) const
+{
+    return attribute.name() == srcdocAttr || HTMLFrameOwnerElement::isHTMLContentAttribute(attribute);
+}
+
 int HTMLFrameElementBase::width()
 {
     document()->updateLayoutIgnorePendingStylesheets();
index 31ab2ab..4e711a4 100644 (file)
@@ -59,6 +59,8 @@ private:
     virtual void setFocus(bool) OVERRIDE;
     
     virtual bool isURLAttribute(const Attribute&) const OVERRIDE;
+    virtual bool isHTMLContentAttribute(const Attribute&) const OVERRIDE;
+
     virtual bool isFrameElementBase() const { return true; }
 
     virtual bool areAuthorShadowsAllowed() const OVERRIDE { return false; }
index 3192d35..0684d01 100644 (file)
@@ -60,7 +60,7 @@ using namespace HTMLNames;
 static inline void setAttributes(Element* element, AtomicHTMLToken* token, ParserContentPolicy parserContentPolicy)
 {
     if (!scriptingContentIsAllowed(parserContentPolicy))
-        element->stripJavaScriptAttributes(token->attributes());
+        element->stripScriptingAttributes(token->attributes());
     element->parserSetAttributes(token->attributes());
 }
 
index d7b4973..40ca751 100644 (file)
@@ -370,7 +370,7 @@ private:
 static inline void setAttributes(Element* element, Vector<Attribute>& attributeVector, ParserContentPolicy parserContentPolicy)
 {
     if (!scriptingContentIsAllowed(parserContentPolicy))
-        element->stripJavaScriptAttributes(attributeVector);
+        element->stripScriptingAttributes(attributeVector);
     element->parserSetAttributes(attributeVector);
 }
 
index 6e42c92..83fb334 100644 (file)
@@ -67,7 +67,7 @@ namespace WebCore {
 static inline void setAttributes(Element* element, Vector<Attribute>& attributeVector, ParserContentPolicy parserContentPolicy)
 {
     if (!scriptingContentIsAllowed(parserContentPolicy))
-        element->stripJavaScriptAttributes(attributeVector);
+        element->stripScriptingAttributes(attributeVector);
     element->parserSetAttributes(attributeVector);
 }