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

content/browser/frame_host/render_frame_host_impl.cc

Index: content/browser/frame_host/render_frame_host_impl.cc
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index cd3a864d50d0e1a705539d3bfec517dfd39ce22b..a506e3f7004e3c2f9ab41769af8178f5dadc6fec 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -2170,6 +2170,12 @@ void RenderFrameHostImpl::OnBeginNavigation(
   BeginNavigationParams validated_begin_params = begin_params;
   GetProcess()->FilterURL(true, &validated_begin_params.searchable_form_url);
 
+  if (!ValidateUploadParams(validated_params)) {
+    bad_message::ReceivedBadMessage(GetProcess(),
+                                    bad_message::RFH_ILLEGAL_UPLOAD_PARAMS);
+    return;
+  }
+

[Łukasz Anforowicz, 2017/05/24 20:24:10]

This is unrelated to the bug you are fixing and therefore optional and can be
done in a follow-up CL.  I am wondering if (here or underneath
frame_tree_node()->navigator()->OnBeginNavigation called below) we could also:

    if (common_params.method != "POST") {
      DCHECK(!common_params.post_data);
      common_params.post_data = nullptr;
    }

PS. I vaguely remember that we cannot do
    if (common_params.method == "POST")
      DCHECK(common_params.url.SchemeIsHTTPOrHTTPS())
because you can post to chrome-extension:// URI (AFAIR under the covers this
will be like a GET - the POST body will be lost).  I forgot other details... :-/

   if (waiting_for_init_) {
     pendinging_navigate_ = base::MakeUnique<PendingNavigation>(
         validated_params, validated_begin_params);
@@ -3926,6 +3932,36 @@ void RenderFrameHostImpl::BeforeUnloadTimeout() {
   SimulateBeforeUnloadAck();
 }
 
+bool RenderFrameHostImpl::ValidateUploadParams(
+    const CommonNavigationParams& common_params) {
+  if (!common_params.post_data.get())
+    return true;
+
+  // Check if the renderer is permitted to upload the requested files.

[Łukasz Anforowicz, 2017/05/24 20:24:10]

Is there any chance the code below can be abstracted away (and shared with
ResourceDispatcherHostImpl::ShouldServiceRequest and
RenderFrameProxyHost::OnOpenURL)?

I am not sure what a good place would be for the shared function - maybe a new
instance method in ChildProcessSecurityPolicyImpl - bool CanReadBody(const
ResourceRequestBodyImpl&) ?

PS. Interestingly, just for the files, there is
ChildProcessSecurityPolicyImpl::CanReadAllFiles,
 but it doesn't handle filesystem things (but I think these don't survive inside
PageState?).

+  const std::vector<ResourceRequestBodyImpl::Element>* uploads =
+      common_params.post_data->elements();
+  std::vector<ResourceRequestBodyImpl::Element>::const_iterator iter;
+  ChildProcessSecurityPolicyImpl* security_policy =
+      ChildProcessSecurityPolicyImpl::GetInstance();
+  for (iter = uploads->begin(); iter != uploads->end(); ++iter) {
+    if (iter->type() == ResourceRequestBodyImpl::Element::TYPE_FILE &&
+        !security_policy->CanReadFile(GetProcess()->GetID(), iter->path())) {
+      return false;
+    }
+    if (iter->type() ==
+        ResourceRequestBodyImpl::Element::TYPE_FILE_FILESYSTEM) {
+      StoragePartition* storage_partition = BrowserContext::GetStoragePartition(
+          GetSiteInstance()->GetBrowserContext(), GetSiteInstance());
+      storage::FileSystemURL url =
+          storage_partition->GetFileSystemContext()->CrackURL(
+              iter->filesystem_url());
+      if (!security_policy->CanReadFileSystemFile(GetProcess()->GetID(), url))
+        return false;
+    }
+  }
+  return true;
+}
+
 #if defined(OS_ANDROID)
 
 class RenderFrameHostImpl::JavaInterfaceProvider