[webkit-changes] [134766] trunk

Thu, 15 Nov 2012 05:03:22 -0800

Title: [134766] trunk
Revision
134766
Author
mk...@chromium.org
Date
2012-11-15 05:04:54 -0800 (Thu, 15 Nov 2012)

Log Message

We should trigger a console warning when we encounter invalid sandbox flags.
https://bugs.webkit.org/show_bug.cgi?id=101956

Reviewed by Adam Barth.

Source/WebCore:

A developer who writes '<iframe sandbox="allowScripts">' probably has
something in mind other than what the browser interprets. In these
situations, we should log a console warning that notes 'allowScripts'
is an invalid sandbox flag ('allow-scripts' is probably what she
meant).

This patch does the simplest thing possible: it throws a warning that
lists the invalid flags encountered for sandbox attributes on iframes,
and for sandbox Content Security Policy directives.

Tests: http/tests/security/contentSecurityPolicy/sandbox-invalid-header.html
       http/tests/security/sandboxed-iframe-invalid.html

* dom/SecurityContext.cpp:
(WebCore::SecurityContext::parseSandboxPolicy):
* dom/SecurityContext.h:
(SecurityContext):
    Accept a new out parameter, invalidTokensErrorMessage. If invalid
    tokens are encountered, build an error message string, and pass it
    back to the caller through this parameter.
* html/HTMLIFrameElement.cpp:
(WebCore::HTMLIFrameElement::parseAttribute):
* page/ContentSecurityPolicy.cpp:
(WebCore::CSPDirectiveList::applySandboxPolicy):
    When applying a sandbox policy, pass a string into
    SecurityContext::parseSandboxPolicy to grab any errors that might
    be encountered, and log a warning in that event.
(WebCore::ContentSecurityPolicy::reportInvalidSandboxFlags):
(WebCore):
* page/ContentSecurityPolicy.h:
    Adding a new method to report invalid sandbox flags.

LayoutTests:

* http/tests/security/contentSecurityPolicy/sandbox-invalid-header-expected.txt: Added.
* http/tests/security/contentSecurityPolicy/sandbox-invalid-header.html: Added.
* http/tests/security/sandboxed-iframe-invalid-expected.txt: Added.
* http/tests/security/sandboxed-iframe-invalid.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (134765 => 134766)


--- trunk/LayoutTests/ChangeLog	2012-11-15 13:00:33 UTC (rev 134765)
+++ trunk/LayoutTests/ChangeLog	2012-11-15 13:04:54 UTC (rev 134766)
@@ -1,3 +1,15 @@
+2012-11-15  Mike West  <mk...@chromium.org>
+
+        We should trigger a console warning when we encounter invalid sandbox flags.
+        https://bugs.webkit.org/show_bug.cgi?id=101956
+
+        Reviewed by Adam Barth.
+
+        * http/tests/security/contentSecurityPolicy/sandbox-invalid-header-expected.txt: Added.
+        * http/tests/security/contentSecurityPolicy/sandbox-invalid-header.html: Added.
+        * http/tests/security/sandboxed-iframe-invalid-expected.txt: Added.
+        * http/tests/security/sandboxed-iframe-invalid.html: Added.
+
 2012-11-15  Keishi Hattori  <kei...@webkit.org>
 
         [Chromium] Add tests for month suggestion picker

Modified: trunk/LayoutTests/fast/frames/sandboxed-iframe-attribute-parsing-expected.txt (134765 => 134766)


--- trunk/LayoutTests/fast/frames/sandboxed-iframe-attribute-parsing-expected.txt	2012-11-15 13:00:33 UTC (rev 134765)
+++ trunk/LayoutTests/fast/frames/sandboxed-iframe-attribute-parsing-expected.txt	2012-11-15 13:04:54 UTC (rev 134766)
@@ -1,3 +1,11 @@
+CONSOLE MESSAGE: line 113: Error while parsing the 'sandbox' attribute: 'För', 'var', 'vers,', 'jag', 'gör,', 'Lovar', 'du', 'en', 'kyss', 'mig', 'giva;', 'Arket', 'fullt', 'jag', 'borde', 'skriva,', 'Mindre', 'har', 'jag', 'skrivit', 'för.', 'Men', 'man', 'måste', 'hålla', 'måtta,', 'Jag', 'med', 'vers,', 'med', 'kyssar', 'du.', 'Låt', 'mig', 'räkna:', 'Där', 'är', 'sju!', 'En', 'därtill', 'det', 'gör', 'mig', 'åtta.', 'Numro', 'åtta', 'är', 'fatal,', 'Greklands', 'sångmör', 'voro', 'nio,', 'Och', 'en', 'svensk', 'därtill', 'gör', 'tio.', '—', 'Elva', 'var', 'apostelns', 'tal,', 'Ty', 'jag', 'räknar', 'icke', 'Judas,', 'Honom,', 'som', 'i', 'vänners', 'lag', 'Kysste', 'falskt;', 'det', 'gör', 'ej', 'jag,', 'Helst', 'när', 'vackra', 'läppar', 'bjudas.', 'Huru', 'står', 'min', 'räkning', 'här?', 'Aderton;', 'det', 'är', 'dock', 'något.', 'Nitton', '—', 'rimmet', 'gör', 'besvär,', 'Därföre', 'jag', 'fyller', 'tjoget.', 'Strofen', 'är', 'ej', 'full', 'som', 'jag,', 'In', 'i', 'hamnen', 'vill', 'jag', 'styra,', 'Därföre', 'till', 'godo', 'tag', 'Denna', 'gång', 'med', 'tjugofyra.', ''Kyssarna'', '('The', 'kisses'),', 'Esaias', 'Tegnér,', '1782-1846', 'int', 'main(void)', '{', 'return', '0;', '}' are invalid sandbox flags.
+CONSOLE MESSAGE: line 131: Error while parsing the 'sandbox' attribute: 'allowscripts' is an invalid sandbox flag.
+CONSOLE MESSAGE: line 135: Error while parsing the 'sandbox' attribute: 'allows-cripts' is an invalid sandbox flag.
+CONSOLE MESSAGE: line 139: Error while parsing the 'sandbox' attribute: '-allow-scripts' is an invalid sandbox flag.
+CONSOLE MESSAGE: line 143: Error while parsing the 'sandbox' attribute: 'allow_scripts' is an invalid sandbox flag.
+CONSOLE MESSAGE: line 147: Error while parsing the 'sandbox' attribute: 'allowScripts' is an invalid sandbox flag.
+CONSOLE MESSAGE: line 151: Error while parsing the 'sandbox' attribute: 'aallow-scripts' is an invalid sandbox flag.
+CONSOLE MESSAGE: line 155: Error while parsing the 'sandbox' attribute: 'allow-scriptss' is an invalid sandbox flag.
 This test case verifies the parsing of the iframe sandbox attribute. Two sets of iframes are used: one where scripting is allowed, and another one where it is disallowed. The test verifies that the allowed frames execute scripts (but other sandboxed properties still apply -- specifically, forms are disabled), and the disallowed ones do not. If successful the test prints "PASS".

Modified: trunk/LayoutTests/fast/frames/sandboxed-iframe-parsing-space-characters-expected.txt (134765 => 134766)


--- trunk/LayoutTests/fast/frames/sandboxed-iframe-parsing-space-characters-expected.txt	2012-11-15 13:00:33 UTC (rev 134765)
+++ trunk/LayoutTests/fast/frames/sandboxed-iframe-parsing-space-characters-expected.txt	2012-11-15 13:04:54 UTC (rev 134766)
@@ -1,6 +1,8 @@
 ALERT: PASS: Form feed is a delimiter.
+CONSOLE MESSAGE: line 41: Error while parsing the 'sandbox' attribute: 'allow-scriptsallow-forms' is an invalid sandbox flag.
 ALERT: PASS: Newline is a delimiter.
 ALERT: PASS: Return is a delimiter.
+CONSOLE MESSAGE: Error while parsing the 'sandbox' attribute: 'allow-scriptsxallow-forms' is an invalid sandbox flag.
 ALERT: PASS: Tab is a delimiter.
 ALERT: PASS: Space is a delimiter character.
 This tests whether we correct parse various space characters in the sandbox attribute.

Added: trunk/LayoutTests/http/tests/security/contentSecurityPolicy/sandbox-invalid-header-expected.txt (0 => 134766)


--- trunk/LayoutTests/http/tests/security/contentSecurityPolicy/sandbox-invalid-header-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/contentSecurityPolicy/sandbox-invalid-header-expected.txt	2012-11-15 13:04:54 UTC (rev 134766)
@@ -0,0 +1,2 @@
+CONSOLE MESSAGE: Error while parsing the 'sandbox' Content Security Policy directive: 'allowScript' is an invalid sandbox flag.
+

Added: trunk/LayoutTests/http/tests/security/contentSecurityPolicy/sandbox-invalid-header.html (0 => 134766)


--- trunk/LayoutTests/http/tests/security/contentSecurityPolicy/sandbox-invalid-header.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/contentSecurityPolicy/sandbox-invalid-header.html	2012-11-15 13:04:54 UTC (rev 134766)
@@ -0,0 +1,6 @@
+<!DOCTYPE html>
+<iframe src=""
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>

Added: trunk/LayoutTests/http/tests/security/sandboxed-iframe-invalid-expected.txt (0 => 134766)


--- trunk/LayoutTests/http/tests/security/sandboxed-iframe-invalid-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/sandboxed-iframe-invalid-expected.txt	2012-11-15 13:04:54 UTC (rev 134766)
@@ -0,0 +1,5 @@
+CONSOLE MESSAGE: line 13: Error while parsing the 'sandbox' attribute: 'allowScripts' is an invalid sandbox flag.
+CONSOLE MESSAGE: line 14: Error while parsing the 'sandbox' attribute: 'allowScripts', 'allowSameOrigin', 'allowFoobarbloop' are invalid sandbox flags.
+Test that an iframe with invalid sandbox flags generates a relevant warning.
+
+

Added: trunk/LayoutTests/http/tests/security/sandboxed-iframe-invalid.html (0 => 134766)


--- trunk/LayoutTests/http/tests/security/sandboxed-iframe-invalid.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/sandboxed-iframe-invalid.html	2012-11-15 13:04:54 UTC (rev 134766)
@@ -0,0 +1,17 @@
+<html>
+<head>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+}
+</script>
+</head>
+
+<body>
+<p>Test that an iframe with invalid sandbox flags generates a relevant warning.</p>
+
+<iframe id="theFrame" sandbox="allowScripts"></iframe>
+<iframe id="theFrame2" sandbox="allowScripts allowSameOrigin allowFoobarbloop"></iframe>
+
+</body>
+</html>

Modified: trunk/LayoutTests/platform/chromium/http/tests/security/sandboxed-iframe-modify-self-expected.txt (134765 => 134766)


--- trunk/LayoutTests/platform/chromium/http/tests/security/sandboxed-iframe-modify-self-expected.txt	2012-11-15 13:00:33 UTC (rev 134765)
+++ trunk/LayoutTests/platform/chromium/http/tests/security/sandboxed-iframe-modify-self-expected.txt	2012-11-15 13:04:54 UTC (rev 134766)
@@ -1,5 +1,3 @@
-CONSOLE MESSAGE: Unsafe _javascript_ attempt to initiate a navigation change for frame with URL http://127.0.0.1:8000/security/sandboxed-iframe-form-top.html from frame with URL http://127.0.0.1:8000/security/resources/sandboxed-iframe-form-top.html.
-
 CONSOLE MESSAGE: Sandbox access violation: Unsafe _javascript_ attempt to access frame with URL http://127.0.0.1:8000/security/sandboxed-iframe-modify-self.html from frame with URL http://127.0.0.1:8000/security/resources/sandboxed-iframe-modify-self.html. The frame requesting access is sandboxed into a unique origin.
 
 This is a "sanity" test case to verify that a sandboxed frame cannot break out of its sandbox by modifying its own sandbox attribute. Two attempts are made:

Modified: trunk/Source/WebCore/ChangeLog (134765 => 134766)


--- trunk/Source/WebCore/ChangeLog	2012-11-15 13:00:33 UTC (rev 134765)
+++ trunk/Source/WebCore/ChangeLog	2012-11-15 13:04:54 UTC (rev 134766)
@@ -1,3 +1,42 @@
+2012-11-15  Mike West  <mk...@chromium.org>
+
+        We should trigger a console warning when we encounter invalid sandbox flags.
+        https://bugs.webkit.org/show_bug.cgi?id=101956
+
+        Reviewed by Adam Barth.
+
+        A developer who writes '<iframe sandbox="allowScripts">' probably has
+        something in mind other than what the browser interprets. In these
+        situations, we should log a console warning that notes 'allowScripts'
+        is an invalid sandbox flag ('allow-scripts' is probably what she
+        meant).
+
+        This patch does the simplest thing possible: it throws a warning that
+        lists the invalid flags encountered for sandbox attributes on iframes,
+        and for sandbox Content Security Policy directives.
+
+        Tests: http/tests/security/contentSecurityPolicy/sandbox-invalid-header.html
+               http/tests/security/sandboxed-iframe-invalid.html
+
+        * dom/SecurityContext.cpp:
+        (WebCore::SecurityContext::parseSandboxPolicy):
+        * dom/SecurityContext.h:
+        (SecurityContext):
+            Accept a new out parameter, invalidTokensErrorMessage. If invalid
+            tokens are encountered, build an error message string, and pass it
+            back to the caller through this parameter.
+        * html/HTMLIFrameElement.cpp:
+        (WebCore::HTMLIFrameElement::parseAttribute):
+        * page/ContentSecurityPolicy.cpp:
+        (WebCore::CSPDirectiveList::applySandboxPolicy):
+            When applying a sandbox policy, pass a string into
+            SecurityContext::parseSandboxPolicy to grab any errors that might
+            be encountered, and log a warning in that event.
+        (WebCore::ContentSecurityPolicy::reportInvalidSandboxFlags):
+        (WebCore):
+        * page/ContentSecurityPolicy.h:
+            Adding a new method to report invalid sandbox flags.
+
 2012-11-15  Kenneth Rohde Christiansen  <kenn...@webkit.org>
 
         Rename member vars in ViewportArgument to match css-device-adapt

Modified: trunk/Source/WebCore/dom/SecurityContext.cpp (134765 => 134766)


--- trunk/Source/WebCore/dom/SecurityContext.cpp	2012-11-15 13:00:33 UTC (rev 134765)
+++ trunk/Source/WebCore/dom/SecurityContext.cpp	2012-11-15 13:04:54 UTC (rev 134766)
@@ -31,6 +31,7 @@
 #include "HTMLParserIdioms.h"
 #include "SecurityOrigin.h"
 #include "WebCoreMemoryInstrumentation.h"
+#include <wtf/text/StringBuilder.h>
 
 namespace WebCore {
 
@@ -84,7 +85,7 @@
     // Subclasses can override this function if the need to do extra work when the security origin changes.
 }
 
-SandboxFlags SecurityContext::parseSandboxPolicy(const String& policy)
+SandboxFlags SecurityContext::parseSandboxPolicy(const String& policy, String& invalidTokensErrorMessage)
 {
     // http://www.w3.org/TR/html5/the-iframe-element.html#attr-iframe-sandbox
     // Parse the unordered set of unique space-separated tokens.
@@ -92,6 +93,8 @@
     const UChar* characters = policy.characters();
     unsigned length = policy.length();
     unsigned start = 0;
+    unsigned numberOfTokenErrors = 0;
+    StringBuilder tokenErrors;
     while (true) {
         while (start < length && isHTMLSpace(characters[start]))
             ++start;
@@ -116,10 +119,27 @@
             flags &= ~SandboxPopups;
         else if (equalIgnoringCase(sandboxToken, "allow-pointer-lock"))
             flags &= ~SandboxPointerLock;
+        else {
+            if (numberOfTokenErrors)
+                tokenErrors.appendLiteral(", '");
+            else
+                tokenErrors.append('\'');
+            tokenErrors.append(sandboxToken);
+            tokenErrors.append('\'');
+            numberOfTokenErrors++;
+        }
 
         start = end + 1;
     }
 
+    if (numberOfTokenErrors) {
+        if (numberOfTokenErrors > 1)
+            tokenErrors.appendLiteral(" are invalid sandbox flags.");
+        else
+            tokenErrors.appendLiteral(" is an invalid sandbox flag.");
+        invalidTokensErrorMessage = tokenErrors.toString();
+    }
+
     return flags;
 }

Modified: trunk/Source/WebCore/dom/SecurityContext.h (134765 => 134766)


--- trunk/Source/WebCore/dom/SecurityContext.h	2012-11-15 13:00:33 UTC (rev 134765)
+++ trunk/Source/WebCore/dom/SecurityContext.h	2012-11-15 13:04:54 UTC (rev 134766)
@@ -71,7 +71,7 @@
     //       that already contains content.
     void setSecurityOrigin(PassRefPtr<SecurityOrigin>);
 
-    static SandboxFlags parseSandboxPolicy(const String& policy);
+    static SandboxFlags parseSandboxPolicy(const String& policy, String& invalidTokensErrorMessage);
 
     virtual void reportMemoryUsage(MemoryObjectInfo*) const;

Modified: trunk/Source/WebCore/html/HTMLIFrameElement.cpp (134765 => 134766)


--- trunk/Source/WebCore/html/HTMLIFrameElement.cpp	2012-11-15 13:00:33 UTC (rev 134765)
+++ trunk/Source/WebCore/html/HTMLIFrameElement.cpp	2012-11-15 13:04:54 UTC (rev 134766)
@@ -32,6 +32,8 @@
 #include "HTMLNames.h"
 #include "NodeRenderingContext.h"
 #include "RenderIFrame.h"
+#include "ScriptableDocumentParser.h"
+#include <wtf/text/TextPosition.h>
 
 namespace WebCore {
 
@@ -85,9 +87,14 @@
             document->addExtraNamedItem(newName);
         }
         m_name = newName;
-    } else if (attribute.name() == sandboxAttr)
-        setSandboxFlags(attribute.isNull() ? SandboxNone : SecurityContext::parseSandboxPolicy(attribute.value()));
-    else if (attribute.name() == seamlessAttr) {
+    } else if (attribute.name() == sandboxAttr) {
+        String invalidTokens;
+        setSandboxFlags(attribute.isNull() ? SandboxNone : SecurityContext::parseSandboxPolicy(attribute.value(), invalidTokens));
+        if (!invalidTokens.isNull()) {
+            int line = document()->scriptableDocumentParser() ? document()->scriptableDocumentParser()->lineNumber().oneBasedInt() : 0;
+            document()->addConsoleMessage(HTMLMessageSource, LogMessageType, ErrorMessageLevel, "Error while parsing the 'sandbox' attribute: " + invalidTokens, document()->url().string(), line);
+        }
+    } else if (attribute.name() == seamlessAttr) {
         // If we're adding or removing the seamless attribute, we need to force the content document to recalculate its StyleResolver.
         if (contentDocument())
             contentDocument()->styleResolverChanged(DeferRecalcStyle);

Modified: trunk/Source/WebCore/page/ContentSecurityPolicy.cpp (134765 => 134766)


--- trunk/Source/WebCore/page/ContentSecurityPolicy.cpp	2012-11-15 13:00:33 UTC (rev 134765)
+++ trunk/Source/WebCore/page/ContentSecurityPolicy.cpp	2012-11-15 13:04:54 UTC (rev 134766)
@@ -1280,7 +1280,10 @@
         return;
     }
     m_haveSandboxPolicy = true;
-    m_policy->enforceSandboxFlags(SecurityContext::parseSandboxPolicy(sandboxPolicy));
+    String invalidTokens;
+    m_policy->enforceSandboxFlags(SecurityContext::parseSandboxPolicy(sandboxPolicy, invalidTokens));
+    if (!invalidTokens.isNull())
+        m_policy->reportInvalidSandboxFlags(invalidTokens);
 }
 
 void CSPDirectiveList::addDirective(const String& name, const String& value)
@@ -1654,6 +1657,11 @@
     logToConsole(message);
 }
 
+void ContentSecurityPolicy::reportInvalidSandboxFlags(const String& invalidFlags) const
+{
+    logToConsole("Error while parsing the 'sandbox' Content Security Policy directive: " + invalidFlags);
+}
+
 void ContentSecurityPolicy::reportInvalidDirectiveValueCharacter(const String& directiveName, const String& value) const
 {
     String message = makeString("The value for Content Security Policy directive '", directiveName, "' contains an invalid character: '", value, "'. Non-whitespace characters outside ASCII 0x21-0x7E must be percent-encoded, as described in RFC 3986, section 2.1: http://tools.ietf.org/html/rfc3986#section-2.1.");

Modified: trunk/Source/WebCore/page/ContentSecurityPolicy.h (134765 => 134766)


--- trunk/Source/WebCore/page/ContentSecurityPolicy.h	2012-11-15 13:00:33 UTC (rev 134765)
+++ trunk/Source/WebCore/page/ContentSecurityPolicy.h	2012-11-15 13:04:54 UTC (rev 134766)
@@ -107,6 +107,7 @@
     void reportInvalidPathCharacter(const String& directiveName, const String& value, const char) const;
     void reportInvalidNonce(const String&) const;
     void reportInvalidPluginTypes(const String&) const;
+    void reportInvalidSandboxFlags(const String&) const;
     void reportInvalidSourceExpression(const String& directiveName, const String& source) const;
     void reportUnsupportedDirective(const String&) const;
     void reportViolation(const String& directiveText, const String& consoleMessage, const KURL& blockedURL, const Vector<KURL>& reportURIs, const String& header, const String& contextURL = String(), const WTF::OrdinalNumber& contextLine = WTF::OrdinalNumber::beforeFirst(), ScriptState* = 0) const;

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to