JavaScript URLs execute in sandboxed iframes
https://bugs.webkit.org/show_bug.cgi?id=71599
Reviewed by Eric Seidel.
Source/WebCore:
This patch fixes the intentional regression I introduced earlier today
by moving the sandbox bits from SecurityOrigin to Document. In the
process, I renamed SecurityOrigin::createEmpty to
SecurityOrigin::createUnique to better align with HTML5 terminology.
* WebCore.exp.in:
* bindings/ScriptControllerBase.cpp:
(WebCore::ScriptController::canExecuteScripts):
* dom/Document.cpp:
(WebCore::Document::setIsViewSource):
(WebCore::Document::initSecurityContext):
* dom/ScriptExecutionContext.cpp:
(WebCore::ScriptExecutionContext::ScriptExecutionContext):
* dom/ScriptExecutionContext.h:
(WebCore::ScriptExecutionContext::sandboxFlags):
(WebCore::ScriptExecutionContext::enforceSandboxFlags):
(WebCore::ScriptExecutionContext::isSandboxed):
* html/HTMLAppletElement.cpp:
(WebCore::HTMLAppletElement::canEmbedJava):
* loader/DocumentWriter.cpp:
(WebCore::DocumentWriter::begin):
* loader/FrameLoader.cpp:
(WebCore::isDocumentSandboxed):
(WebCore::FrameLoader::addHTTPOriginIfNeeded):
* loader/PolicyChecker.cpp:
(WebCore::PolicyChecker::checkNewWindowPolicy):
* loader/SubframeLoader.cpp:
(WebCore::SubframeLoader::requestPlugin):
* page/SecurityOrigin.cpp:
(WebCore::SecurityOrigin::SecurityOrigin):
(WebCore::SecurityOrigin::create):
(WebCore::SecurityOrigin::createUnique):
* page/SecurityOrigin.h:
LayoutTests:
Update results to show progression.
* fast/frames/sandboxed-iframe-scripting-expected.txt:
* fast/frames/sandboxed-iframe-scripting.html:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@99347 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index db0941f..60e8c11 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2011-11-04 Adam Barth <abarth@webkit.org>
+
+ JavaScript URLs execute in sandboxed iframes
+ https://bugs.webkit.org/show_bug.cgi?id=71599
+
+ Reviewed by Eric Seidel.
+
+ Update results to show progression.
+
+ * fast/frames/sandboxed-iframe-scripting-expected.txt:
+ * fast/frames/sandboxed-iframe-scripting.html:
+
2011-11-04 Stephen Chenney <schenney@chromium.org>
Crash in ScrollAnimator.cpp
diff --git a/LayoutTests/fast/frames/sandboxed-iframe-scripting-expected.txt b/LayoutTests/fast/frames/sandboxed-iframe-scripting-expected.txt
index 0394b44..fd4dd39 100644
--- a/LayoutTests/fast/frames/sandboxed-iframe-scripting-expected.txt
+++ b/LayoutTests/fast/frames/sandboxed-iframe-scripting-expected.txt
@@ -4,7 +4,7 @@
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
-FAIL disallowedExecuted should be 0. Was 1.
+PASS disallowedExecuted is 0
PASS allowedExecuted is 3
PASS successfullyParsed is true
diff --git a/LayoutTests/fast/frames/sandboxed-iframe-scripting.html b/LayoutTests/fast/frames/sandboxed-iframe-scripting.html
index 69877a0..cdb6570 100644
--- a/LayoutTests/fast/frames/sandboxed-iframe-scripting.html
+++ b/LayoutTests/fast/frames/sandboxed-iframe-scripting.html
@@ -26,10 +26,6 @@
<iframe sandbox="allow-same-origin allow-scripts"
src="data:text/html,<script> alert('PASS: Executed script in data URL'); </script>">
</iframe>
- <!-- This case execute script even though it shouldn't. The problem is that
- the iframe starts out as about:blank, which inherits the parent documents
- SecurityOrigin, along with its frozen sandbox bits. To fix this case,
- we'll need to move the sandbox bits from SecurityOrigin to Document. --!>
<iframe sandbox="allow-same-origin"
src="javascript: ++window.top.disallowedExecuted;">
</iframe>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index a97c722f..150c953 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,44 @@
+2011-11-04 Adam Barth <abarth@webkit.org>
+
+ JavaScript URLs execute in sandboxed iframes
+ https://bugs.webkit.org/show_bug.cgi?id=71599
+
+ Reviewed by Eric Seidel.
+
+ This patch fixes the intentional regression I introduced earlier today
+ by moving the sandbox bits from SecurityOrigin to Document. In the
+ process, I renamed SecurityOrigin::createEmpty to
+ SecurityOrigin::createUnique to better align with HTML5 terminology.
+
+ * WebCore.exp.in:
+ * bindings/ScriptControllerBase.cpp:
+ (WebCore::ScriptController::canExecuteScripts):
+ * dom/Document.cpp:
+ (WebCore::Document::setIsViewSource):
+ (WebCore::Document::initSecurityContext):
+ * dom/ScriptExecutionContext.cpp:
+ (WebCore::ScriptExecutionContext::ScriptExecutionContext):
+ * dom/ScriptExecutionContext.h:
+ (WebCore::ScriptExecutionContext::sandboxFlags):
+ (WebCore::ScriptExecutionContext::enforceSandboxFlags):
+ (WebCore::ScriptExecutionContext::isSandboxed):
+ * html/HTMLAppletElement.cpp:
+ (WebCore::HTMLAppletElement::canEmbedJava):
+ * loader/DocumentWriter.cpp:
+ (WebCore::DocumentWriter::begin):
+ * loader/FrameLoader.cpp:
+ (WebCore::isDocumentSandboxed):
+ (WebCore::FrameLoader::addHTTPOriginIfNeeded):
+ * loader/PolicyChecker.cpp:
+ (WebCore::PolicyChecker::checkNewWindowPolicy):
+ * loader/SubframeLoader.cpp:
+ (WebCore::SubframeLoader::requestPlugin):
+ * page/SecurityOrigin.cpp:
+ (WebCore::SecurityOrigin::SecurityOrigin):
+ (WebCore::SecurityOrigin::create):
+ (WebCore::SecurityOrigin::createUnique):
+ * page/SecurityOrigin.h:
+
2011-11-04 Joseph Pecoraro <pecoraro@apple.com>
Potential Unused Param Build Issue
diff --git a/Source/WebCore/WebCore.exp.in b/Source/WebCore/WebCore.exp.in
index 49951f0..1bbbbdb 100644
--- a/Source/WebCore/WebCore.exp.in
+++ b/Source/WebCore/WebCore.exp.in
@@ -390,7 +390,7 @@
__ZN7WebCore14SecurityOrigin32removeOriginAccessWhitelistEntryERKS0_RKN3WTF6StringES6_b
__ZN7WebCore14SecurityOrigin40setDomainRelaxationForbiddenForURLSchemeEbRKN3WTF6StringE
__ZN7WebCore14SecurityOrigin6createERKN3WTF6StringES4_i
-__ZN7WebCore14SecurityOrigin6createERKNS_4KURLEi
+__ZN7WebCore14SecurityOrigin6createERKNS_4KURLEb
__ZN7WebCore14StorageTracker7originsERN3WTF6VectorINS1_6RefPtrINS_14SecurityOriginEEELm0EEE
__ZN7WebCore14StorageTracker7trackerEv
__ZN7WebCore14StorageTracker9setClientEPNS_20StorageTrackerClientE
diff --git a/Source/WebCore/bindings/ScriptControllerBase.cpp b/Source/WebCore/bindings/ScriptControllerBase.cpp
index 63b976e..570f964 100644
--- a/Source/WebCore/bindings/ScriptControllerBase.cpp
+++ b/Source/WebCore/bindings/ScriptControllerBase.cpp
@@ -37,7 +37,7 @@
bool ScriptController::canExecuteScripts(ReasonForCallingCanExecuteScripts reason)
{
- if (m_frame->document() && m_frame->document()->securityOrigin()->isSandboxed(SandboxScripts))
+ if (m_frame->document() && m_frame->document()->isSandboxed(SandboxScripts))
return false;
if (m_frame->document() && m_frame->document()->isViewSource()) {
diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
index ca989f19..948187d 100644
--- a/Source/WebCore/dom/Document.cpp
+++ b/Source/WebCore/dom/Document.cpp
@@ -1724,7 +1724,7 @@
if (!m_isViewSource)
return;
- ScriptExecutionContext::setSecurityOrigin(SecurityOrigin::create(url(), SandboxOrigin));
+ ScriptExecutionContext::setSecurityOrigin(SecurityOrigin::createUnique());
}
void Document::createStyleSelector()
@@ -4394,7 +4394,7 @@
// No source for a security context.
// This can occur via document.implementation.createDocument().
m_cookieURL = KURL(ParsedURLString, "");
- ScriptExecutionContext::setSecurityOrigin(SecurityOrigin::createEmpty());
+ ScriptExecutionContext::setSecurityOrigin(SecurityOrigin::createUnique());
ScriptExecutionContext::setContentSecurityPolicy(ContentSecurityPolicy::create(this));
return;
}
@@ -4402,7 +4402,8 @@
// In the common case, create the security context from the currently
// loading URL with a fresh content security policy.
m_cookieURL = m_url;
- ScriptExecutionContext::setSecurityOrigin(SecurityOrigin::create(m_url, m_frame->loader()->sandboxFlags()));
+ ScriptExecutionContext::enforceSandboxFlags(m_frame->loader()->sandboxFlags());
+ ScriptExecutionContext::setSecurityOrigin(SecurityOrigin::create(m_url, isSandboxed(SandboxOrigin)));
ScriptExecutionContext::setContentSecurityPolicy(ContentSecurityPolicy::create(this));
if (SecurityOrigin::allowSubstituteDataAccessToLocal()) {
diff --git a/Source/WebCore/dom/ScriptExecutionContext.cpp b/Source/WebCore/dom/ScriptExecutionContext.cpp
index 1452f5b..030e0aa 100644
--- a/Source/WebCore/dom/ScriptExecutionContext.cpp
+++ b/Source/WebCore/dom/ScriptExecutionContext.cpp
@@ -89,7 +89,8 @@
}
ScriptExecutionContext::ScriptExecutionContext()
- : m_iteratingActiveDOMObjects(false)
+ : m_sandboxFlags(SandboxNone)
+ , m_iteratingActiveDOMObjects(false)
, m_inDestructor(false)
, m_inDispatchErrorEvent(false)
#if ENABLE(SQL_DATABASE)
diff --git a/Source/WebCore/dom/ScriptExecutionContext.h b/Source/WebCore/dom/ScriptExecutionContext.h
index c39d4e7..1f6d00c 100644
--- a/Source/WebCore/dom/ScriptExecutionContext.h
+++ b/Source/WebCore/dom/ScriptExecutionContext.h
@@ -29,6 +29,7 @@
#include "ActiveDOMObject.h"
#include "ConsoleTypes.h"
+#include "FrameLoaderTypes.h"
#include "KURL.h"
#include <wtf/Forward.h>
#include <wtf/HashMap.h>
@@ -94,8 +95,12 @@
virtual void disableEval() = 0;
SecurityOrigin* securityOrigin() const { return m_securityOrigin.get(); }
+ SandboxFlags sandboxFlags() const { return m_sandboxFlags; }
ContentSecurityPolicy* contentSecurityPolicy() { return m_contentSecurityPolicy.get(); }
+ void enforceSandboxFlags(SandboxFlags mask) { m_sandboxFlags |= mask; }
+ bool isSandboxed(SandboxFlags mask) const { return m_sandboxFlags & mask; }
+
bool sanitizeScriptError(String& errorMessage, int& lineNumber, String& sourceURL);
void reportException(const String& errorMessage, int lineNumber, const String& sourceURL, PassRefPtr<ScriptCallStack>);
virtual void addMessage(MessageSource, MessageType, MessageLevel, const String& message, unsigned lineNumber, const String& sourceURL, PassRefPtr<ScriptCallStack>) = 0;
@@ -201,6 +206,7 @@
void closeMessagePorts();
+ SandboxFlags m_sandboxFlags;
RefPtr<SecurityOrigin> m_securityOrigin;
RefPtr<ContentSecurityPolicy> m_contentSecurityPolicy;
diff --git a/Source/WebCore/html/HTMLAppletElement.cpp b/Source/WebCore/html/HTMLAppletElement.cpp
index 1ba3448..cf6cc84 100644
--- a/Source/WebCore/html/HTMLAppletElement.cpp
+++ b/Source/WebCore/html/HTMLAppletElement.cpp
@@ -164,7 +164,7 @@
bool HTMLAppletElement::canEmbedJava() const
{
- if (document()->securityOrigin()->isSandboxed(SandboxPlugins))
+ if (document()->isSandboxed(SandboxPlugins))
return false;
Settings* settings = document()->settings();
diff --git a/Source/WebCore/loader/DocumentWriter.cpp b/Source/WebCore/loader/DocumentWriter.cpp
index 478d479..e9c4f9a 100644
--- a/Source/WebCore/loader/DocumentWriter.cpp
+++ b/Source/WebCore/loader/DocumentWriter.cpp
@@ -119,7 +119,7 @@
// If the new document is for a Plugin but we're supposed to be sandboxed from Plugins,
// then replace the document with one whose parser will ignore the incoming data (bug 39323)
- if (document->isPluginDocument() && document->securityOrigin()->isSandboxed(SandboxPlugins))
+ if (document->isPluginDocument() && document->isSandboxed(SandboxPlugins))
document = SinkDocument::create(m_frame, url);
// FIXME: Do we need to consult the content security policy here about blocked plug-ins?
diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp
index 41379bb..28d4680 100644
--- a/Source/WebCore/loader/FrameLoader.cpp
+++ b/Source/WebCore/loader/FrameLoader.cpp
@@ -163,7 +163,7 @@
//
static bool isDocumentSandboxed(Frame* frame, SandboxFlags mask)
{
- return frame->document() && frame->document()->securityOrigin()->isSandboxed(mask);
+ return frame->document() && frame->document()->isSandboxed(mask);
}
FrameLoader::FrameLoader(Frame* frame, FrameLoaderClient* client)
@@ -2546,7 +2546,7 @@
if (origin.isEmpty()) {
// If we don't know what origin header to attach, we attach the value
// for an empty origin.
- request.setHTTPOrigin(SecurityOrigin::createEmpty()->toString());
+ request.setHTTPOrigin(SecurityOrigin::createUnique()->toString());
return;
}
diff --git a/Source/WebCore/loader/PolicyChecker.cpp b/Source/WebCore/loader/PolicyChecker.cpp
index fde8600..5b3416e 100644
--- a/Source/WebCore/loader/PolicyChecker.cpp
+++ b/Source/WebCore/loader/PolicyChecker.cpp
@@ -93,7 +93,7 @@
void PolicyChecker::checkNewWindowPolicy(const NavigationAction& action, NewWindowPolicyDecisionFunction function,
const ResourceRequest& request, PassRefPtr<FormState> formState, const String& frameName, void* argument)
{
- if (m_frame->document() && m_frame->document()->securityOrigin()->isSandboxed(SandboxPopups))
+ if (m_frame->document() && m_frame->document()->isSandboxed(SandboxPopups))
return continueAfterNavigationPolicy(PolicyIgnore);
m_callback.set(request, formState, frameName, action, function, argument);
diff --git a/Source/WebCore/loader/SubframeLoader.cpp b/Source/WebCore/loader/SubframeLoader.cpp
index 7f48b9d..e02b07b 100644
--- a/Source/WebCore/loader/SubframeLoader.cpp
+++ b/Source/WebCore/loader/SubframeLoader.cpp
@@ -112,7 +112,7 @@
return false;
if (m_frame->document()) {
- if (m_frame->document()->securityOrigin()->isSandboxed(SandboxPlugins))
+ if (m_frame->document()->isSandboxed(SandboxPlugins))
return false;
if (!m_frame->document()->contentSecurityPolicy()->allowObjectFromSource(url))
return false;
diff --git a/Source/WebCore/page/SecurityOrigin.cpp b/Source/WebCore/page/SecurityOrigin.cpp
index a53ec89..906b9e5 100644
--- a/Source/WebCore/page/SecurityOrigin.cpp
+++ b/Source/WebCore/page/SecurityOrigin.cpp
@@ -66,12 +66,11 @@
return schemes.contains(scheme);
}
-SecurityOrigin::SecurityOrigin(const KURL& url, SandboxFlags sandboxFlags)
- : m_sandboxFlags(sandboxFlags)
- , m_protocol(url.protocol().isNull() ? "" : url.protocol().lower())
+SecurityOrigin::SecurityOrigin(const KURL& url, bool forceUnique)
+ : m_protocol(url.protocol().isNull() ? "" : url.protocol().lower())
, m_host(url.host().isNull() ? "" : url.host().lower())
, m_port(url.port())
- , m_isUnique(isSandboxed(SandboxOrigin) || SchemeRegistry::shouldTreatURLSchemeAsNoAccess(m_protocol))
+ , m_isUnique(forceUnique || SchemeRegistry::shouldTreatURLSchemeAsNoAccess(m_protocol))
, m_universalAccess(false)
, m_domainWasSetInDOM(false)
, m_enforceFilePathSeparation(false)
@@ -131,8 +130,7 @@
}
SecurityOrigin::SecurityOrigin(const SecurityOrigin* other)
- : m_sandboxFlags(other->m_sandboxFlags)
- , m_protocol(other->m_protocol.isolatedCopy())
+ : m_protocol(other->m_protocol.isolatedCopy())
, m_host(other->m_host.isolatedCopy())
, m_encodedHost(other->m_encodedHost.isolatedCopy())
, m_domain(other->m_domain.isolatedCopy())
@@ -152,16 +150,18 @@
return m_protocol.isEmpty();
}
-PassRefPtr<SecurityOrigin> SecurityOrigin::create(const KURL& url, SandboxFlags sandboxFlags)
+PassRefPtr<SecurityOrigin> SecurityOrigin::create(const KURL& url, bool forceUnique)
{
if (!url.isValid())
- return adoptRef(new SecurityOrigin(KURL(), sandboxFlags));
- return adoptRef(new SecurityOrigin(url, sandboxFlags));
+ return adoptRef(new SecurityOrigin(blankURL(), forceUnique));
+ return adoptRef(new SecurityOrigin(url, forceUnique));
}
-PassRefPtr<SecurityOrigin> SecurityOrigin::createEmpty()
+PassRefPtr<SecurityOrigin> SecurityOrigin::createUnique()
{
- return create(KURL());
+ RefPtr<SecurityOrigin> origin = create(KURL());
+ ASSERT(origin->isUnique());
+ return origin.release();
}
PassRefPtr<SecurityOrigin> SecurityOrigin::isolatedCopy()
@@ -478,7 +478,7 @@
PassRefPtr<SecurityOrigin> SecurityOrigin::create(const String& protocol, const String& host, int port)
{
if (port < 0 || port > MaxAllowedPort)
- create(KURL());
+ createUnique();
String decodedHost = decodeURLEscapeSequences(host);
return create(KURL(KURL(), protocol + "://" + host + ":" + String::number(port)));
}
diff --git a/Source/WebCore/page/SecurityOrigin.h b/Source/WebCore/page/SecurityOrigin.h
index 564599e..59588cb 100644
--- a/Source/WebCore/page/SecurityOrigin.h
+++ b/Source/WebCore/page/SecurityOrigin.h
@@ -40,11 +40,12 @@
class SecurityOrigin : public ThreadSafeRefCounted<SecurityOrigin> {
public:
+ static PassRefPtr<SecurityOrigin> create(const KURL&, bool forceUnique = false);
+ static PassRefPtr<SecurityOrigin> createUnique();
+
static PassRefPtr<SecurityOrigin> createFromDatabaseIdentifier(const String&);
static PassRefPtr<SecurityOrigin> createFromString(const String&);
static PassRefPtr<SecurityOrigin> create(const String& protocol, const String& host, int port);
- static PassRefPtr<SecurityOrigin> create(const KURL&, SandboxFlags = SandboxNone);
- static PassRefPtr<SecurityOrigin> createEmpty();
// Create a deep copy of this SecurityOrigin. This method is useful
// when marshalling a SecurityOrigin to another thread.
@@ -114,9 +115,6 @@
// WARNING: This is an extremely powerful ability. Use with caution!
void grantUniversalAccess();
- bool isSandboxed(SandboxFlags mask) const { return m_sandboxFlags & mask; }
- SandboxFlags sandboxFlags() const { return m_sandboxFlags; }
-
bool canAccessDatabase() const { return !isUnique(); }
bool canAccessLocalStorage() const { return !isUnique(); }
bool canAccessCookies() const { return !isUnique(); }
@@ -198,7 +196,7 @@
static void resetOriginAccessWhitelists();
private:
- SecurityOrigin(const KURL&, SandboxFlags);
+ explicit SecurityOrigin(const KURL&, bool forceUnique);
explicit SecurityOrigin(const SecurityOrigin*);
// FIXME: Rename this function to something more semantic.
@@ -207,7 +205,6 @@
bool isAccessWhiteListed(const SecurityOrigin*) const;
bool isAccessToURLWhiteListed(const KURL&) const;
- SandboxFlags m_sandboxFlags;
String m_protocol;
String m_host;
mutable String m_encodedHost;
