RenderFrameProxyHost::OnOpenURL needs to validate resource request body.

content/browser/cross_site_transfer_browsertest.cc

Index: content/browser/cross_site_transfer_browsertest.cc
diff --git a/content/browser/cross_site_transfer_browsertest.cc b/content/browser/cross_site_transfer_browsertest.cc
index 601909e0dc2ac1c70cd7a71f8744f5a47eee6428..192ec2194faf825e3c2e9e14d68062aa37880743 100644
--- a/content/browser/cross_site_transfer_browsertest.cc
+++ b/content/browser/cross_site_transfer_browsertest.cc
@@ -15,6 +15,7 @@
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/navigation_handle.h"
+#include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/resource_dispatcher_host_delegate.h"
 #include "content/public/browser/resource_throttle.h"
@@ -515,6 +516,95 @@ IN_PROC_BROWSER_TEST_P(CrossSiteTransferTest, PostWithFileData) {
               ::testing::HasSubstr("form-data; name=\"file\""));
 }
 
+// Test that verifies that if navigation originator doesn't have access to a
+// file, then no access is granted after a cross-process transfer of POST data.
+//
+// This test is somewhat similar to
+// http/tests/navigation/form-targets-cross-site-frame-post.html layout test
+// except that it 1) tests with files, 2) simulates a malicious scenario and 3)
+// verifies file acecss (all of these 3 things are not possible with layout

[alexmos, 2017/05/25 18:17:33]

nit: access

[Łukasz Anforowicz, 2017/05/25 19:56:01]

Done.

+// tests).
+//
+// This test is very similar to CrossSiteTransferTest.PostWithFileData above,
+// except that it simulates a malicious form / POST originator.
+IN_PROC_BROWSER_TEST_P(CrossSiteTransferTest, MaliciousPostWithFileData) {
+  // Navigate to the page with form that posts via 307 redirection to
+  // |redirect_target_url| (cross-site from |form_url|).  Using 307 (rather than
+  // 302) redirection is important to preserve the HTTP method and POST body.
+  GURL form_url(embedded_test_server()->GetURL(
+      "main.com", "/form_that_posts_cross_site.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), form_url));
+  WebContents* form_contents = shell()->web_contents();
+
+  // Create a new window and make it a target of the form from |form_contents|.
+  GURL initial_target_url(
+      embedded_test_server()->GetURL("initial-target.com", "/title1.html"));
+  WebContentsAddedObserver new_contents_observer;
+  std::string script = base::StringPrintf(
+      R"( window.open('%s', 'form-target');
+          document.getElementById('file-form').target = 'form-target';
+      )",
+      initial_target_url.spec().c_str());
+  EXPECT_TRUE(ExecuteScript(form_contents, script));
+  WebContents* target_contents = new_contents_observer.GetWebContents();
+  WaitForLoadStop(target_contents);
+
+  // Verify the current location and process placement of |target_contents|.
+  EXPECT_EQ(initial_target_url, target_contents->GetLastCommittedURL());
+  EXPECT_NE(target_contents->GetMainFrame()->GetProcess()->GetID(),
+            form_contents->GetMainFrame()->GetProcess()->GetID());
+
+  // Prepare a file to upload.
+  base::ThreadRestrictions::ScopedAllowIO allow_io_for_temp_dir;
+  base::ScopedTempDir temp_dir;
+  base::FilePath file_path;
+  std::string file_content("test-file-content");
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+  ASSERT_TRUE(base::CreateTemporaryFileInDir(temp_dir.GetPath(), &file_path));
+  ASSERT_LT(
+      0, base::WriteFile(file_path, file_content.data(), file_content.size()));
+
+  // Fill out the form to refer to the test file.
+  std::unique_ptr<FileChooserDelegate> delegate(
+      new FileChooserDelegate(file_path));
+  form_contents->SetDelegate(delegate.get());
+  EXPECT_TRUE(
+      ExecuteScript(form_contents, "document.getElementById('file').click();"));
+  EXPECT_TRUE(delegate->file_chosen());
+
+  // Simulate a malicious situation, where the renderer doesn't really have
+  // access to the file.
+  ChildProcessSecurityPolicyImpl* security_policy =
+      ChildProcessSecurityPolicyImpl::GetInstance();
+  security_policy->RevokeAllPermissionsForFile(
+      form_contents->GetMainFrame()->GetProcess()->GetID(), file_path);
+  EXPECT_FALSE(security_policy->CanReadFile(
+      form_contents->GetMainFrame()->GetProcess()->GetID(), file_path));
+  EXPECT_FALSE(security_policy->CanReadFile(
+      target_contents->GetMainFrame()->GetProcess()->GetID(), file_path));
+
+  // Submit the form.
+  RenderProcessHostWatcher process_exit_observer(
+      form_contents->GetMainFrame()->GetProcess(),
+      RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
+  EXPECT_TRUE(ExecuteScript(
+      form_contents,
+      "setTimeout(\n"
+      "  function() { document.getElementById('file-form').submit(); },\n"
+      "  0);"));
+  process_exit_observer.Wait();
+
+  // The target frame should still be at the original location - the malicious
+  // navigation should have been stopped.
+  EXPECT_EQ(initial_target_url, target_contents->GetLastCommittedURL());
+
+  // Both processes still shouldn't have access.
+  EXPECT_FALSE(security_policy->CanReadFile(
+      form_contents->GetMainFrame()->GetProcess()->GetID(), file_path));
+  EXPECT_FALSE(security_policy->CanReadFile(
+      target_contents->GetMainFrame()->GetProcess()->GetID(), file_path));
+}
+
 INSTANTIATE_TEST_CASE_P(CrossSiteTransferTest,
                         CrossSiteTransferTest,
                         ::testing::Values(TestParameter::LOADING_WITHOUT_MOJO,