Implement allow-popups for iframe@sandbox

https://bugs.webkit.org/show_bug.cgi?id=66505 Reviewed by Eric Seidel. Source/WebCore: There's been some discussion in the HTML working group about adding an allow-popups directive to the iframe sandbox. Microsoft has added it to IE10 platform preview and is fairly adamant about this feature because it's needed by one or their products that's planning to use iframe sandbox. Hixie says he'll add it to the spec once we implement it, so here's our implementation. (See discussion in the W3C linked in the bug for more details.) Tests: http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control.html http/tests/security/popup-allowed-by-sandbox-is-sandboxed.html http/tests/security/popup-allowed-by-sandbox-when-allowed.html * html/HTMLIFrameElement.cpp: (WebCore::HTMLIFrameElement::parseMappedAttribute): * loader/FrameLoader.cpp: (WebCore::FrameLoader::setOpener): (WebCore::createWindow): * loader/FrameLoader.h: (WebCore::FrameLoader::forceSandboxFlags): * loader/FrameLoaderTypes.h: * loader/PolicyChecker.cpp: (WebCore::PolicyChecker::checkNewWindowPolicy): * page/SecurityOrigin.cpp: (WebCore::SecurityOrigin::parseSandboxPolicy): * page/SecurityOrigin.h: (WebCore::SecurityOrigin::sandboxFlags): * svg/graphics/SVGImage.cpp: (WebCore::SVGImage::dataChanged): LayoutTests: Test that the allow-popups directive works as expected. Note: no-popup-from-sandbox.html verifies that we still block popups without the directive. * http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control-expected.txt: Added. * http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control.html: Added. * http/tests/security/popup-allowed-by-sandbox-is-sandboxed-expected.txt: Added. * http/tests/security/popup-allowed-by-sandbox-is-sandboxed.html: Added. * http/tests/security/popup-allowed-by-sandbox-when-allowed-expected.txt: Added. * http/tests/security/popup-allowed-by-sandbox-when-allowed.html: Added. Canonical link: https://commits.webkit.org/87729@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@99138 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Nov 3, 2011 · 78f8706 · 78f8706
1 parent 7c43e7a
commit 78f8706
Show file tree

Hide file tree

Showing 16 changed files with 170 additions and 41 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,21 @@
+2011-11-02 Adam Barth <abarth@webkit.org>
+
+ Implement allow-popups for iframe@sandbox
+ https://bugs.webkit.org/show_bug.cgi?id=66505
+
+ Reviewed by Eric Seidel.
+
+ Test that the allow-popups directive works as expected. Note:
+ no-popup-from-sandbox.html verifies that we still block popups without
+ the directive.
+
+ * http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control-expected.txt: Added.
+ * http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control.html: Added.
+ * http/tests/security/popup-allowed-by-sandbox-is-sandboxed-expected.txt: Added.
+ * http/tests/security/popup-allowed-by-sandbox-is-sandboxed.html: Added.
+ * http/tests/security/popup-allowed-by-sandbox-when-allowed-expected.txt: Added.
+ * http/tests/security/popup-allowed-by-sandbox-when-allowed.html: Added.
+
 2011-11-02 Sam Weinig <sam@webkit.org>
 
  Object.getOwnPropertyDescriptor() does not retrieve the getter/setter from a property on the window that has been overridden with a getter/setter/

diff --git a/LayoutTests/http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control-expected.txt b/LayoutTests/http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control-expected.txt
@@ -0,0 +1,6 @@
+ALERT: /PASS/
+To run this test outside of DumpRenderTree, please disable your popup blocker!
+
+If you change this test, please be sure to change popup-allowed-by-sandbox-is-sandboxed.html as well!
+
+
diff --git a/LayoutTests/http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control.html b/LayoutTests/http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control.html
@@ -0,0 +1,16 @@
+<script>
+if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+ layoutTestController.setCanOpenWindows(true);
+ layoutTestController.setCloseRemainingWindowsWhenComplete(true);
+}
+</script>
+<p>To run this test outside of DumpRenderTree, please disable your popup blocker!</p>
+<p>If you change this test, please be sure to change popup-allowed-by-sandbox-is-sandboxed.html as well!</p>
+<iframe sandbox="allow-scripts allow-popups allow-forms"
+ src="data:text/html,
+ <script>
+ var win = window.open('data:text/html,<form action=javascript:alert(/PASS/) ><input type=submit></form><script>document.forms[0].submit(); if (window.layoutTestController) layoutTestController.notifyDone();<\/script>', '_blank');
+ </script>"
+ ></iframe>
diff --git a/LayoutTests/http/tests/security/popup-allowed-by-sandbox-is-sandboxed-expected.txt b/LayoutTests/http/tests/security/popup-allowed-by-sandbox-is-sandboxed-expected.txt
@@ -0,0 +1,7 @@
+To run this test outside of DumpRenderTree, please disable your popup blocker!
+
+If you change this test, please be sure to change popup-allowed-by-sandbox-is-sandboxed-control.html as well!
+
+This test passes if it doesn't alert FAIL.
+
+
diff --git a/LayoutTests/http/tests/security/popup-allowed-by-sandbox-is-sandboxed.html b/LayoutTests/http/tests/security/popup-allowed-by-sandbox-is-sandboxed.html
@@ -0,0 +1,17 @@
+<script>
+if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+ layoutTestController.setCanOpenWindows(true);
+ layoutTestController.setCloseRemainingWindowsWhenComplete(true);
+}
+</script>
+<p>To run this test outside of DumpRenderTree, please disable your popup blocker!</p>
+<p>If you change this test, please be sure to change popup-allowed-by-sandbox-is-sandboxed-control.html as well!</p>
+<p>This test passes if it doesn't alert FAIL.</p>
+<iframe sandbox="allow-scripts allow-popups"
+ src="data:text/html,
+ <script>
+ var win = window.open('data:text/html,<form action=javascript:alert(/FAIL/) ><input type=submit></form><script>document.forms[0].submit(); if (window.layoutTestController) layoutTestController.notifyDone();<\/script>', '_blank');
+ </script>"
+ ></iframe>
diff --git a/LayoutTests/http/tests/security/popup-allowed-by-sandbox-when-allowed-expected.txt b/LayoutTests/http/tests/security/popup-allowed-by-sandbox-when-allowed-expected.txt
@@ -0,0 +1,4 @@
+ALERT: PASS
+To run this test outside of DumpRenderTree, please disable your popup blocker!
+
+
diff --git a/LayoutTests/http/tests/security/popup-allowed-by-sandbox-when-allowed.html b/LayoutTests/http/tests/security/popup-allowed-by-sandbox-when-allowed.html
@@ -0,0 +1,16 @@
+<script>
+if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+ layoutTestController.setCanOpenWindows(true);
+ layoutTestController.setCloseRemainingWindowsWhenComplete(true);
+}
+</script>
+<p>To run this test outside of DumpRenderTree, please disable your popup blocker!</p>
+<iframe sandbox="allow-scripts allow-popups"
+ src="data:text/html,
+ <script>
+ var win = window.open('data:text/html,<script>if (window.layoutTestController) layoutTestController.notifyDone();<\/script>', '_blank');
+ alert(win ? 'PASS' : 'FAIL');
+ </script>"
+ ></iframe>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,39 @@
+2011-11-02 Adam Barth <abarth@webkit.org>
+
+ Implement allow-popups for iframe@sandbox
+ https://bugs.webkit.org/show_bug.cgi?id=66505
+
+ Reviewed by Eric Seidel.
+
+ There's been some discussion in the HTML working group about adding an
+ allow-popups directive to the iframe sandbox. Microsoft has added it
+ to IE10 platform preview and is fairly adamant about this feature
+ because it's needed by one or their products that's planning to use
+ iframe sandbox. Hixie says he'll add it to the spec once we implement
+ it, so here's our implementation. (See discussion in the W3C linked in
+ the bug for more details.)
+
+ Tests: http/tests/security/popup-allowed-by-sandbox-is-sandboxed-control.html
+ http/tests/security/popup-allowed-by-sandbox-is-sandboxed.html
+ http/tests/security/popup-allowed-by-sandbox-when-allowed.html
+
+ * html/HTMLIFrameElement.cpp:
+ (WebCore::HTMLIFrameElement::parseMappedAttribute):
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::setOpener):
+ (WebCore::createWindow):
+ * loader/FrameLoader.h:
+ (WebCore::FrameLoader::forceSandboxFlags):
+ * loader/FrameLoaderTypes.h:
+ * loader/PolicyChecker.cpp:
+ (WebCore::PolicyChecker::checkNewWindowPolicy):
+ * page/SecurityOrigin.cpp:
+ (WebCore::SecurityOrigin::parseSandboxPolicy):
+ * page/SecurityOrigin.h:
+ (WebCore::SecurityOrigin::sandboxFlags):
+ * svg/graphics/SVGImage.cpp:
+ (WebCore::SVGImage::dataChanged):
+
 2011-11-02 Sam Weinig <sam@webkit.org>
 
  Remove the ability to generate custom lookupGetter/lookupSetter functions,

diff --git a/Source/WebCore/html/HTMLIFrameElement.cpp b/Source/WebCore/html/HTMLIFrameElement.cpp
@@ -32,6 +32,7 @@
 #include "HTMLNames.h"
 #include "NodeRenderingContext.h"
 #include "RenderIFrame.h"
+#include "SecurityOrigin.h"
 
 namespace WebCore {
 
@@ -68,42 +69,6 @@ bool HTMLIFrameElement::mapToEntry(const QualifiedName& attrName, MappedAttribut
  return HTMLFrameElementBase::mapToEntry(attrName, result);
 }
 
-static SandboxFlags parseSandboxAttribute(Attribute* attribute)
-{
- if (attribute->isNull())
- return SandboxNone;
-
- // Parse the unordered set of unique space-separated tokens.
- SandboxFlags flags = SandboxAll;
- const UChar* characters = attribute->value().characters();
- unsigned length = attribute->value().length();
- unsigned start = 0;
- while (true) {
- while (start < length && isASCIISpace(characters[start]))
- ++start;
- if (start >= length)
- break;
- unsigned end = start + 1;
- while (end < length && !isASCIISpace(characters[end]))
- ++end;
-
- // Turn off the corresponding sandbox flag if it's set as "allowed".
- String sandboxToken = String(characters + start, end - start);
- if (equalIgnoringCase(sandboxToken, "allow-same-origin"))
- flags &= ~SandboxOrigin;
- else if (equalIgnoringCase(sandboxToken, "allow-forms"))
- flags &= ~SandboxForms;
- else if (equalIgnoringCase(sandboxToken, "allow-scripts"))
- flags &= ~SandboxScripts;
- else if (equalIgnoringCase(sandboxToken, "allow-top-navigation"))
- flags &= ~SandboxTopNavigation;
-
- start = end + 1;
- }
-
- return flags;
-}
-
 void HTMLIFrameElement::parseMappedAttribute(Attribute* attr)
 {
  if (attr->name() == widthAttr)
@@ -127,7 +92,7 @@ void HTMLIFrameElement::parseMappedAttribute(Attribute* attr)
  // Add a rule that nulls out our border width.
  addCSSLength(attr, CSSPropertyBorderWidth, "0");
  } else if (attr->name() == sandboxAttr)
- setSandboxFlags(parseSandboxAttribute(attr));
+ setSandboxFlags(attr->isNull() ? SandboxNone : SecurityOrigin::parseSandboxPolicy(attr->value()));
  else
  HTMLFrameElementBase::parseMappedAttribute(attr);
 }

diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp
@@ -945,8 +945,12 @@ void FrameLoader::setOpener(Frame* opener)
  m_opener->loader()->m_openedFrames.remove(m_frame);
  if (opener)
  opener->loader()->m_openedFrames.add(m_frame);
+
  m_opener = opener;
 
+ if (m_opener && !m_frame->tree()->parent())
+ forceSandboxFlags(m_opener->document()->securityOrigin()->sandboxFlags());
+
  if (m_frame->document()) {
  m_frame->document()->initSecurityContext();
  m_frame->domWindow()->setSecurityOrigin(m_frame->document()->securityOrigin());
@@ -3268,7 +3272,7 @@ Frame* createWindow(Frame* openerFrame, Frame* lookupFrame, const FrameLoadReque
  }
 
  // Sandboxed frames cannot open new auxiliary browsing contexts.
- if (isDocumentSandboxed(openerFrame, SandboxNavigation))
+ if (isDocumentSandboxed(openerFrame, SandboxPopups))
  return 0;
 
  // FIXME: Setting the referrer should be the caller's responsibility.

diff --git a/Source/WebCore/loader/FrameLoader.h b/Source/WebCore/loader/FrameLoader.h
@@ -215,7 +215,7 @@ class FrameLoader {
  SandboxFlags sandboxFlags() const { return m_sandboxFlags; }
  // The following sandbox flags will be forced, regardless of changes to
  // the sandbox attribute of any parent frames.
- void setForcedSandboxFlags(SandboxFlags flags) { m_forcedSandboxFlags = flags; m_sandboxFlags |= flags; }
+ void forceSandboxFlags(SandboxFlags flags) { m_forcedSandboxFlags |= flags; m_sandboxFlags |= flags; }
 
  // Mixed content related functions.
  static bool isMixedContent(SecurityOrigin* context, const KURL&);

diff --git a/Source/WebCore/loader/FrameLoaderTypes.h b/Source/WebCore/loader/FrameLoaderTypes.h
@@ -100,6 +100,7 @@ namespace WebCore {
  SandboxForms = 1 << 3,
  SandboxScripts = 1 << 4,
  SandboxTopNavigation = 1 << 5,
+ SandboxPopups = 1 << 6,
  SandboxAll = -1 // Mask with all bits set to 1.
  };
 

diff --git a/Source/WebCore/loader/PolicyChecker.cpp b/Source/WebCore/loader/PolicyChecker.cpp
@@ -93,7 +93,7 @@ void PolicyChecker::checkNavigationPolicy(const ResourceRequest& request, Docume
 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(SandboxNavigation))
+ if (m_frame->document() && m_frame->document()->securityOrigin()->isSandboxed(SandboxPopups))
  return continueAfterNavigationPolicy(PolicyIgnore);
 
  m_callback.set(request, formState, frameName, action, function, argument);

diff --git a/Source/WebCore/page/SecurityOrigin.cpp b/Source/WebCore/page/SecurityOrigin.cpp
@@ -543,6 +543,41 @@ bool SecurityOrigin::shouldHideReferrer(const KURL& url, const String& referrer)
  return !URLIsSecureURL;
 }
 
+SandboxFlags SecurityOrigin::parseSandboxPolicy(const String& policy)
+{
+ // Parse the unordered set of unique space-separated tokens.
+ SandboxFlags flags = SandboxAll;
+ const UChar* characters = policy.characters();
+ unsigned length = policy.length();
+ unsigned start = 0;
+ while (true) {
+ while (start < length && isASCIISpace(characters[start]))
+ ++start;
+ if (start >= length)
+ break;
+ unsigned end = start + 1;
+ while (end < length && !isASCIISpace(characters[end]))
+ ++end;
+
+ // Turn off the corresponding sandbox flag if it's set as "allowed".
+ String sandboxToken = String(characters + start, end - start);
+ if (equalIgnoringCase(sandboxToken, "allow-same-origin"))
+ flags &= ~SandboxOrigin;
+ else if (equalIgnoringCase(sandboxToken, "allow-forms"))
+ flags &= ~SandboxForms;
+ else if (equalIgnoringCase(sandboxToken, "allow-scripts"))
+ flags &= ~SandboxScripts;
+ else if (equalIgnoringCase(sandboxToken, "allow-top-navigation"))
+ flags &= ~SandboxTopNavigation;
+ else if (equalIgnoringCase(sandboxToken, "allow-popups"))
+ flags &= ~SandboxPopups;
+
+ start = end + 1;
+ }
+
+ return flags;
+}
+
 void SecurityOrigin::setLocalLoadPolicy(LocalLoadPolicy policy)
 {
  localLoadPolicy = policy;

diff --git a/Source/WebCore/page/SecurityOrigin.h b/Source/WebCore/page/SecurityOrigin.h
@@ -56,6 +56,7 @@ class SecurityOrigin : public ThreadSafeRefCounted<SecurityOrigin> {
  void setDomainFromDOM(const String& newDomain);
  bool domainWasSetInDOM() const { return m_domainWasSetInDOM; }
 
+ // FIXME: This should move to SchemeRegistry.
  static void setDomainRelaxationForbiddenForURLScheme(bool forbidden, const String&);
  static bool isDomainRelaxationForbiddenForURLScheme(const String&);
 
@@ -114,6 +115,7 @@ class SecurityOrigin : public ThreadSafeRefCounted<SecurityOrigin> {
  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(); }
@@ -178,6 +180,8 @@ class SecurityOrigin : public ThreadSafeRefCounted<SecurityOrigin> {
  // (and whether it was set) but considering the host. It is used for postMessage.
  bool isSameSchemeHostPort(const SecurityOrigin*) const;
 
+ static SandboxFlags parseSandboxPolicy(const String& policy);
+
  static bool shouldHideReferrer(const KURL&, const String& referrer);
 
  enum LocalLoadPolicy {

diff --git a/Source/WebCore/svg/graphics/SVGImage.cpp b/Source/WebCore/svg/graphics/SVGImage.cpp
@@ -313,7 +313,7 @@ bool SVGImage::dataChanged(bool allDataReceived)
  frame->setView(FrameView::create(frame.get()));
  frame->init();
  FrameLoader* loader = frame->loader();
- loader->setForcedSandboxFlags(SandboxAll);
+ loader->forceSandboxFlags(SandboxAll);
 
  frame->view()->setCanHaveScrollbars(false); // SVG Images will always synthesize a viewBox, if it's not available, and thus never see scrollbars.
  frame->view()->setTransparent(true); // SVG Images are transparent.