Verify all files in the request body are accessible by the renderer process.

content/browser/browser_side_navigation_browsertest.cc

 // Copyright 2014 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 <stdint.h>
 
 #include "base/command_line.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/frame_host/navigation_handle_impl.h"
 #include "content/browser/frame_host/navigation_request.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/common/site_isolation_policy.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/url_constants.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"
 #include "content/shell/browser/shell.h"
 #include "content/shell/browser/shell_network_delegate.h"
+#include "content/test/content_browser_test_utils_internal.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/test/url_request/url_request_failed_job.h"
 #include "url/gurl.h"
 
 namespace content {
 
 class BrowserSideNavigationBrowserTest : public ContentBrowserTest {
  public:
   BrowserSideNavigationBrowserTest() {}
@@ skipping 280 matching lines @@
   ASSERT_TRUE(root->navigation_request());
   EXPECT_EQ(GURL(),
             root->navigation_request()->navigation_handle()->GetReferrer().url);
 
   // The navigation should commit without being blocked.
   EXPECT_TRUE(manager.WaitForResponse());
   manager.WaitForNavigationFinished();
   EXPECT_EQ(kInsecureUrl, shell()->web_contents()->GetLastCommittedURL());
 }
 
+// Test to verify that an exploited renderer process trying to upload a file
+// it hasn't been explicitly granted permissions to is correctly terminated.
+// TODO(nasko): This test case belongs better in
+// security_exploit_browsertest.cc, so move it there once PlzNavigate is on

[ncarter (slow), 2017/05/24 21:24:38]

If we're no longer using pwnmessagerecieved, do we still want to move this to
security_exploit_browsertest?

[nasko, 2017/05/24 21:43:58]

Yeah, no need to move. Removed.

+// by default.
+IN_PROC_BROWSER_TEST_F(BrowserSideNavigationBrowserTest,
+                       PostUploadIllegalFilePath) {
+  GURL form_url(
+      embedded_test_server()->GetURL("/form_that_posts_to_echoall.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), form_url));
+
+  RenderFrameHostImpl* rfh = static_cast<RenderFrameHostImpl*>(
+      shell()->web_contents()->GetMainFrame());
+
+  // Prepare a file for the upload form.
+  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));
+  shell()->web_contents()->SetDelegate(delegate.get());
+  EXPECT_TRUE(ExecuteScript(shell()->web_contents(),
+                            "document.getElementById('file').click();"));
+  EXPECT_TRUE(delegate->file_chosen());
+
+  // Ensure that the process is allowed to access to the chosen file and
+  // does not have access to the other file name.
+  EXPECT_TRUE(ChildProcessSecurityPolicyImpl::GetInstance()->CanReadFile(
+      rfh->GetProcess()->GetID(), file_path));
+
+  // Revoke the access to the file and submit the form. The renderer process
+  // should be terminanted.

[ncarter (slow), 2017/05/24 21:24:38]

"terminanted" typo

[nasko, 2017/05/24 21:43:58]

Done.

+  RenderProcessHostWatcher process_exit_observer(
+      rfh->GetProcess(), RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
+  ChildProcessSecurityPolicyImpl* security_policy =
+      ChildProcessSecurityPolicyImpl::GetInstance();
+  security_policy->RevokeAllPermissionsForFile(rfh->GetProcess()->GetID(),
+                                               file_path);
+  EXPECT_TRUE(
+      ExecuteScript(shell(), "document.getElementById('file-form').submit();"));

[ncarter (slow), 2017/05/24 21:24:38]

Possibly paranoid: could the wait and EXPECT_TRUE here be a source of flakiness?

In particular, it's not clear to me why the FrameHostMsg_BeginNavigation IPC
(which causes the process kill) would necessarily arrive after the
FrameHostMsg_DomOperationResponse ?

because, in js, the submit() call seems to happen before the send() call.

[nasko, 2017/05/24 21:43:58]

Rewritten to ensure response is sent back before the submit() happens.

+  process_exit_observer.Wait();
+}
+
 }  // namespace content