Bug 634834 - Throw an error if JSON stringification during history.push/replaceState navigates the page. r=bz, a=blocking
authorJustin Lebar <justin.lebar@gmail.com>
Fri, 18 Feb 2011 16:15:42 -0800
changeset 62848 b5fdead607d4f54b8de872e6cbbbc9fb53249444
parent 62847 ffca7d5d2810fc0842a50f45c078b2458717ba0f
child 62849 328483e1b820019df619c0f22f570ae3901d1fe5
push idunknown
push userunknown
push dateunknown
reviewersbz, blocking
bugs634834
milestone2.0b12pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 634834 - Throw an error if JSON stringification during history.push/replaceState navigates the page. r=bz, a=blocking
docshell/base/nsDocShell.cpp
docshell/test/Makefile.in
docshell/test/file_bug634834.html
docshell/test/test_bug634834.html
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -9637,37 +9637,60 @@ nsDocShell::AddState(nsIVariant *aData, 
     // active content viewer.  Since EvictContentViewers at the end of step 5
     // might run script, we can't just put a script blocker around the critical
     // section.
     //
     // Note that we completely ignore the aTitle parameter.
 
     nsresult rv;
 
-    nsCOMPtr<nsIDocument> document = do_GetInterface(GetAsSupports(this));
-    NS_ENSURE_TRUE(document, NS_ERROR_FAILURE);
-
-    // Step 1: Clone aData by getting its JSON representation
+    // Step 1: Clone aData by getting its JSON representation.
+    //
+    // StringifyJSValVariant might cause arbitrary JS to run, and this code
+    // might navigate the page we're on, potentially to a different origin! (bug
+    // 634834)  To protect against this, we abort if our principal changes due
+    // to the stringify call.
     nsString dataStr;
-    rv = StringifyJSValVariant(aData, dataStr);
-    NS_ENSURE_SUCCESS(rv, rv);
+    {
+        nsCOMPtr<nsIDocument> origDocument =
+            do_GetInterface(GetAsSupports(this));
+        if (!origDocument)
+            return NS_ERROR_DOM_SECURITY_ERR;
+        nsCOMPtr<nsIPrincipal> origPrincipal = origDocument->NodePrincipal();
+
+        rv = StringifyJSValVariant(aData, dataStr);
+        NS_ENSURE_SUCCESS(rv, rv);
+
+        nsCOMPtr<nsIDocument> newDocument =
+            do_GetInterface(GetAsSupports(this));
+        if (!newDocument)
+            return NS_ERROR_DOM_SECURITY_ERR;
+        nsCOMPtr<nsIPrincipal> newPrincipal = newDocument->NodePrincipal();
+
+        PRBool principalsEqual = PR_FALSE;
+        origPrincipal->Equals(newPrincipal, &principalsEqual);
+        NS_ENSURE_TRUE(principalsEqual, NS_ERROR_DOM_SECURITY_ERR);
+    }
 
     // Check that the state object isn't too long.
     // Default max length: 640k chars.
     PRInt32 maxStateObjSize = 0xA0000;
     if (mPrefs) {
         mPrefs->GetIntPref("browser.history.maxStateObjectSize",
                            &maxStateObjSize);
     }
     if (maxStateObjSize < 0) {
         maxStateObjSize = 0;
     }
     NS_ENSURE_TRUE(dataStr.Length() <= (PRUint32)maxStateObjSize,
                    NS_ERROR_ILLEGAL_VALUE);
 
+    nsCOMPtr<nsIDocument> document = do_GetInterface(GetAsSupports(this));
+    NS_ENSURE_TRUE(document, NS_ERROR_FAILURE);
+
     // Step 2: Resolve aURL
     PRBool equalURIs = PR_TRUE;
     nsCOMPtr<nsIURI> oldURI = mCurrentURI;
     nsCOMPtr<nsIURI> newURI;
     if (aURL.Length() == 0) {
         newURI = mCurrentURI;
     }
     else {
--- a/docshell/test/Makefile.in
+++ b/docshell/test/Makefile.in
@@ -92,16 +92,18 @@ include $(topsrcdir)/config/rules.mk
 		file_bug580069_2.sjs \
 		test_bug590573.html \
 		file_bug590573_1.html \
 		file_bug590573_2.html \
 		test_bug615501_1.html \
 		test_bug615501_2.html \
 		test_bug615501_3.html \
 		file_bug615501.html \
+		test_bug634834.html \
+		file_bug634834.html \
 		$(NULL)
 
 ifeq ($(MOZ_WIDGET_TOOLKIT),cocoa)
 _TEST_FILES += \
 		test_bug511449.html \
 		file_bug511449.html \
 		$(NULL)
 endif
new file mode 100644
--- /dev/null
+++ b/docshell/test/file_bug634834.html
@@ -0,0 +1,5 @@
+<html>
+<body>
+Nothing to see here; just an empty page.
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/docshell/test/test_bug634834.html
@@ -0,0 +1,53 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=634834
+-->
+<head>
+  <title>Test for Bug 634834</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=634834">Mozilla Bug 634834</a>
+
+<iframe id='iframe' src='file_bug634834.html' onload='iframe_loaded()'></iframe>
+
+<script type='application/javascript;version=1.7'>
+SimpleTest.waitForExplicitFinish();
+
+function iframe_loaded() {
+  var loadedAfterPushstate = false;
+  $('iframe').onload = function() {
+    loadedAfterPushstate = true;
+  }
+
+  var obj = { name: 'name' };
+  obj.__defineGetter__('a', function() {
+    $('iframe').contentWindow.location = 'http://example.com';
+
+    // Wait until we've loaded example.com.
+    do {
+        var r = new XMLHttpRequest();
+        r.open("GET", location.href, false);
+        r.overrideMimeType("text/plain");
+        try { r.send(null); }
+        catch (e) {}
+    } while (!loadedAfterPushstate);
+  });
+
+  try {
+    $('iframe').contentWindow.history.pushState(obj, '');
+    ok(false, 'pushState should throw exception.');
+  }
+  catch(e) {
+    ok(true, 'pushState threw an exception.');
+  }
+  SimpleTest.finish();
+}
+
+</script>
+</body>
+</html>