Sandboxing view source documents is ineffective
authorabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Aug 2012 23:59:52 +0000 (23:59 +0000)
committerabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Aug 2012 23:59:52 +0000 (23:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=93660

Reviewed by Eric Seidel.

Prior to this patch, Document::setIsViewSource changed the
SecurityOrigin object on Document but didn't update the copy on
DOMWindow. As a consequence, the security checks that use the copy on
DOMWindow didn't notice the change and acted as if the document wasn't
sandboxed.

This bug wasn't present for most view source documents because
HTMLViewSourceDocument sets the flag in its constructor. However, for
view source documents created by the XMLTreeViewer, the bit was set at
the end of parsing rather than during construction.

This mechansim is really more of a mitigation than an important
security check, and I was tempted to remove the sandboxing entirely
given that sandboxing a document at the end of parsing isn't overly
effective anyway. However, we can worry about that issue in a future
patch.

For the time being, this patch just synchronizes DOMWindow and
Document's copy of the SecurityOrigin. The long-term solution here, of
course, is to keep working on fixing
https://bugs.webkit.org/show_bug.cgi?id=75793, which this patch gets us
one (small) step closer to fixing.

* dom/Document.cpp:
(WebCore::Document::setIsViewSource):

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

LayoutTests/xmlviewer/extensions-api-expected.txt [moved from LayoutTests/http/tests/xmlviewer/extensions-api-expected.txt with 100% similarity]
LayoutTests/xmlviewer/extensions-api.html [moved from LayoutTests/http/tests/xmlviewer/extensions-api.html with 100% similarity]
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp

index 2739dbc..21e1ebc 100644 (file)
@@ -1,3 +1,36 @@
+2012-08-09  Adam Barth  <abarth@webkit.org>
+
+        Sandboxing view source documents is ineffective
+        https://bugs.webkit.org/show_bug.cgi?id=93660
+
+        Reviewed by Eric Seidel.
+
+        Prior to this patch, Document::setIsViewSource changed the
+        SecurityOrigin object on Document but didn't update the copy on
+        DOMWindow. As a consequence, the security checks that use the copy on
+        DOMWindow didn't notice the change and acted as if the document wasn't
+        sandboxed.
+
+        This bug wasn't present for most view source documents because
+        HTMLViewSourceDocument sets the flag in its constructor. However, for
+        view source documents created by the XMLTreeViewer, the bit was set at
+        the end of parsing rather than during construction.
+
+        This mechansim is really more of a mitigation than an important
+        security check, and I was tempted to remove the sandboxing entirely
+        given that sandboxing a document at the end of parsing isn't overly
+        effective anyway. However, we can worry about that issue in a future
+        patch.
+
+        For the time being, this patch just synchronizes DOMWindow and
+        Document's copy of the SecurityOrigin. The long-term solution here, of
+        course, is to keep working on fixing
+        https://bugs.webkit.org/show_bug.cgi?id=75793, which this patch gets us
+        one (small) step closer to fixing.
+
+        * dom/Document.cpp:
+        (WebCore::Document::setIsViewSource):
+
 2012-08-09  Benjamin Poulain  <bpoulain@apple.com>
 
         Append the unit in place when generating the text value of a CSSPrimitiveValue
index c86b42a..ed9eefe 100644 (file)
@@ -2043,6 +2043,7 @@ void Document::setIsViewSource(bool isViewSource)
         return;
 
     setSecurityOrigin(SecurityOrigin::createUnique());
+    didUpdateSecurityOrigin();
 }
 
 void Document::combineCSSFeatureFlags()