RenderFrameProxyHost::OnOpenURL needs to validate resource request body.

content/browser/cross_site_transfer_browsertest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <memory>
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/stringprintf.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #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"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/content_browser_test_utils.h"
 #include "content/public/test/navigation_handle_observer.h"
 #include "content/public/test/test_navigation_observer.h"
@@ skipping 480 matching lines @@
       "window.domAutomationController.send("
       "document.getElementsByTagName('pre')[0].innerText);",
       &actual_page_body));
   EXPECT_THAT(actual_page_body, ::testing::HasSubstr(file_content));
   EXPECT_THAT(actual_page_body,
               ::testing::HasSubstr(file_path.BaseName().AsUTF8Unsafe()));
   EXPECT_THAT(actual_page_body,
               ::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 is a regression test for https://crbug.com/726067.
+//
+// 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
+// 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) {
+  // The initial test window is a named form target.
+  GURL initial_target_url(
+      embedded_test_server()->GetURL("initial-target.com", "/title1.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), initial_target_url));
+  WebContents* target_contents = shell()->web_contents();
+  EXPECT_TRUE(ExecuteScript(target_contents, "window.name = 'form-target';"));
+
+  // Create a new window containing a form targeting |target_contents|.
+  GURL form_url(embedded_test_server()->GetURL(
+      "main.com", "/form_that_posts_cross_site.html"));
+  WebContentsAddedObserver new_contents_observer;
+  EXPECT_TRUE(ExecuteScript(target_contents, "window.open('" + form_url.spec() +

[alexmos, 2017/05/25 23:44:05]

optional: There's a OpenPopup in content_browser_test_utils_internal.h which
could simplify this.

[Łukasz Anforowicz, 2017/05/26 00:05:14]

Done.

+                                                 "', 'form-window');"));

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

I've rearranged the order of windows - now the window.open-ed window (the one
that shows "on top" in Android) contains the form - this allows the test to
select a file into the form.

I've also considered testing with an OOPIF, but in the original OOPIF-based test
the subframe is lost when the parent frame is killed (making it difficult to
verify "the target frame should still be at the original location").  Hmmm...
not that I think about it more, I guess this can be fixed by also putting the
form into another subframe.  At any rate, I think the current, window.open-based
test is also okay.

+  WebContents* form_contents = new_contents_observer.GetWebContents();
+  WaitForLoadStop(form_contents);
+  EXPECT_TRUE(ExecuteScript(
+      form_contents,
+      "document.getElementById('file-form').target = 'form-target';"));
+
+  // Verify the current locations and process placement of |target_contents|
+  // and |form_contents|.
+  EXPECT_EQ(initial_target_url, target_contents->GetLastCommittedURL());
+  EXPECT_EQ(form_url, form_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->Focus();
+  form_contents->SetDelegate(delegate.get());
+  EXPECT_TRUE(
+      ExecuteScript(form_contents, "document.getElementById('file').click();"));
+  EXPECT_TRUE(delegate->file_chosen());

[alexmos, 2017/05/25 23:44:06]

Might as well sanity-check that at this point,
EXPECT_TRUE(security_policy->CanReadFile(form_contents->GetMainFrame()->GetProcess()->GetID(),
file_path));

[Łukasz Anforowicz, 2017/05/26 00:05:14]

Done.

+
+  // 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 and wait until the malicious renderer gets killed.
+  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();
+  EXPECT_FALSE(process_exit_observer.did_exit_normally());
+
+  // 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,
                                           TestParameter::LOADING_WITH_MOJO));
 
 }  // namespace content