- Revision
- 147281
- Author
- [email protected]
- Date
- 2013-03-30 11:31:52 -0700 (Sat, 30 Mar 2013)
Log Message
Cross-Origin copy&paste / drag&drop allowing XSS via srcdoc attribute.
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.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (147280 => 147281)
--- trunk/LayoutTests/ChangeLog 2013-03-30 18:22:52 UTC (rev 147280)
+++ trunk/LayoutTests/ChangeLog 2013-03-30 18:31:52 UTC (rev 147281)
@@ -1,5 +1,16 @@
2013-03-30 Tom Sepez <[email protected]>
+ 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 <[email protected]>
+
View-source iframes are dangerous (and not very useful).
https://bugs.webkit.org/show_bug.cgi?id=113345
Modified: trunk/LayoutTests/editing/pasteboard/paste-noscript-expected.txt (147280 => 147281)
--- trunk/LayoutTests/editing/pasteboard/paste-noscript-expected.txt 2013-03-30 18:22:52 UTC (rev 147280)
+++ trunk/LayoutTests/editing/pasteboard/paste-noscript-expected.txt 2013-03-30 18:31:52 UTC (rev 147281)
@@ -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 @@
<a id="anchor2">Hello</a>
<iframe id="iframe1" src="" 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="<script>var x = 1;</script>" 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="" formaction="_javascript_:sayHello()" style="width: 200px; height: 150px; background-color:#cee;">This is a form<br><img src="" 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=""
Modified: trunk/LayoutTests/editing/pasteboard/paste-noscript.html (147280 => 147281)
--- trunk/LayoutTests/editing/pasteboard/paste-noscript.html 2013-03-30 18:22:52 UTC (rev 147280)
+++ trunk/LayoutTests/editing/pasteboard/paste-noscript.html 2013-03-30 18:31:52 UTC (rev 147281)
@@ -16,6 +16,7 @@
<a id="anchor1" href=""
<a id="anchor2" href=""
<iframe id="iframe1" src="" 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="" formaction="_javascript_:sayHello()" style="width: 200px; height: 150px; background-color:#cee;">This is a form<br><img src="" formaction="_javascript_:sayHello()">Submit.</button></form>
</div>
<div id="pastehere" contenteditable="true">
@@ -25,7 +26,7 @@
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("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");
Modified: trunk/Source/WebCore/ChangeLog (147280 => 147281)
--- trunk/Source/WebCore/ChangeLog 2013-03-30 18:22:52 UTC (rev 147280)
+++ trunk/Source/WebCore/ChangeLog 2013-03-30 18:31:52 UTC (rev 147281)
@@ -1,5 +1,44 @@
2013-03-30 Tom Sepez <[email protected]>
+ 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 <[email protected]>
+
View-source iframes are dangerous (and not very useful).
https://bugs.webkit.org/show_bug.cgi?id=113345
Modified: trunk/Source/WebCore/dom/Element.cpp (147280 => 147281)
--- trunk/Source/WebCore/dom/Element.cpp 2013-03-30 18:22:52 UTC (rev 147280)
+++ trunk/Source/WebCore/dom/Element.cpp 2013-03-30 18:31:52 UTC (rev 147281)
@@ -1039,29 +1039,18 @@
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;
+ return isURLAttribute(attribute) && protocolIsJavaScript(stripLeadingAndTrailingHTMLSpaces(attribute.value()));
}
-bool Element::isJavaScriptAttribute(const Attribute& attribute)
+void Element::stripScriptingAttributes(Vector<Attribute>& attributeVector) const
{
- if (isEventHandlerAttribute(attribute))
- return true;
- if (isJavaScriptURLAttribute(attribute))
- return true;
- return false;
-}
-
-void Element::stripJavaScriptAttributes(Vector<Attribute>& attributeVector)
-{
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)
Modified: trunk/Source/WebCore/dom/Element.h (147280 => 147281)
--- trunk/Source/WebCore/dom/Element.h 2013-03-30 18:22:52 UTC (rev 147280)
+++ trunk/Source/WebCore/dom/Element.h 2013-03-30 18:31:52 UTC (rev 147281)
@@ -380,7 +380,8 @@
// 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 @@
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 @@
void createRendererIfNeeded();
- bool isJavaScriptAttribute(const Attribute&);
- bool isJavaScriptURLAttribute(const Attribute&);
+ bool isJavaScriptURLAttribute(const Attribute&) const;
RefPtr<ElementData> m_elementData;
};
Modified: trunk/Source/WebCore/html/HTMLFrameElementBase.cpp (147280 => 147281)
--- trunk/Source/WebCore/html/HTMLFrameElementBase.cpp 2013-03-30 18:22:52 UTC (rev 147280)
+++ trunk/Source/WebCore/html/HTMLFrameElementBase.cpp 2013-03-30 18:31:52 UTC (rev 147281)
@@ -222,6 +222,11 @@
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();
Modified: trunk/Source/WebCore/html/HTMLFrameElementBase.h (147280 => 147281)
--- trunk/Source/WebCore/html/HTMLFrameElementBase.h 2013-03-30 18:22:52 UTC (rev 147280)
+++ trunk/Source/WebCore/html/HTMLFrameElementBase.h 2013-03-30 18:31:52 UTC (rev 147281)
@@ -59,6 +59,8 @@
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; }
Modified: trunk/Source/WebCore/html/parser/HTMLConstructionSite.cpp (147280 => 147281)
--- trunk/Source/WebCore/html/parser/HTMLConstructionSite.cpp 2013-03-30 18:22:52 UTC (rev 147280)
+++ trunk/Source/WebCore/html/parser/HTMLConstructionSite.cpp 2013-03-30 18:31:52 UTC (rev 147281)
@@ -60,7 +60,7 @@
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());
}
Modified: trunk/Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp (147280 => 147281)
--- trunk/Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp 2013-03-30 18:22:52 UTC (rev 147280)
+++ trunk/Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp 2013-03-30 18:31:52 UTC (rev 147281)
@@ -370,7 +370,7 @@
static inline void setAttributes(Element* element, Vector<Attribute>& attributeVector, ParserContentPolicy parserContentPolicy)
{
if (!scriptingContentIsAllowed(parserContentPolicy))
- element->stripJavaScriptAttributes(attributeVector);
+ element->stripScriptingAttributes(attributeVector);
element->parserSetAttributes(attributeVector);
}
Modified: trunk/Source/WebCore/xml/parser/XMLDocumentParserQt.cpp (147280 => 147281)
--- trunk/Source/WebCore/xml/parser/XMLDocumentParserQt.cpp 2013-03-30 18:22:52 UTC (rev 147280)
+++ trunk/Source/WebCore/xml/parser/XMLDocumentParserQt.cpp 2013-03-30 18:31:52 UTC (rev 147281)
@@ -67,7 +67,7 @@
static inline void setAttributes(Element* element, Vector<Attribute>& attributeVector, ParserContentPolicy parserContentPolicy)
{
if (!scriptingContentIsAllowed(parserContentPolicy))
- element->stripJavaScriptAttributes(attributeVector);
+ element->stripScriptingAttributes(attributeVector);
element->parserSetAttributes(attributeVector);
}
