RenderFrameProxyHost::OnOpenURL needs to validate resource request body.

content/browser/resource_request_body_browser_utils.cc

Index: content/browser/resource_request_body_browser_utils.cc
diff --git a/content/browser/resource_request_body_browser_utils.cc b/content/browser/resource_request_body_browser_utils.cc
new file mode 100644
index 0000000000000000000000000000000000000000..d1b521a05a04152220f2e5121d94f5d740f3f87c
--- /dev/null
+++ b/content/browser/resource_request_body_browser_utils.cc
@@ -0,0 +1,80 @@
+// Copyright 2017 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 "content/browser/resource_request_body_browser_utils.h"
+
+#include "content/browser/child_process_security_policy_impl.h"
+#include "content/browser/site_instance_impl.h"
+#include "content/common/resource_request_body_impl.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/storage_partition.h"
+#include "storage/browser/fileapi/file_system_context.h"
+
+namespace content {
+
+namespace {
+
+bool CanReadRequestBody(int child_id,
+                        const storage::FileSystemContext* file_system_context,
+                        const scoped_refptr<ResourceRequestBodyImpl>& body) {

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

This one looks like a ChildProcessSecurityPolicyImpl kind of method - any
thoughts about putting it there?  Looks like we've already got similar helpers,
e.g. ChildProcessSecurityPolicyImpl::CanReadAllFiles().

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

nick@: Any concerns with making CanReadRequestBody an instance method of
ChildProcessSecurityPolicyImpl?

- It uses storage::FileSystemContext::CrackURL and
BrowserContext::GetStoragePartition and
storage::StoragePartition->GetFileSystemContext, but this might be fine, given
that child_process_security_policy_impl.cc already includes things from
//storage.

- Some time ago (*) you were concerned about making
PrepareDropDataForChildProcess an instance method of
ChildProcessSecurityPolicyImpl, but this seems different.

(*)
https://codereview.chromium.org/2830743004/diff/40001/content/browser/child_p...

[ncarter (slow), 2017/05/26 21:23:11]

My gut sense: it doesn't totally feel like it fits in with the other methods,
but it's easier to use than the alternative, so I'm okay with it. lgtm

+  if (!body)
+    return true;
+
+  ChildProcessSecurityPolicyImpl* security_policy =
+      ChildProcessSecurityPolicyImpl::GetInstance();
+  for (const ResourceRequestBodyImpl::Element& element : *body->elements()) {
+    switch (element.type()) {
+      case ResourceRequestBodyImpl::Element::TYPE_FILE:
+        if (!security_policy->CanReadFile(child_id, element.path()))
+          return false;
+        break;
+
+      case ResourceRequestBodyImpl::Element::TYPE_FILE_FILESYSTEM:
+        if (!security_policy->CanReadFileSystemFile(
+                child_id,
+                file_system_context->CrackURL(element.filesystem_url())))
+          return false;
+        break;
+
+      case ResourceRequestBodyImpl::Element::TYPE_DISK_CACHE_ENTRY:
+        // TYPE_DISK_CACHE_ENTRY can't be sent via IPC according to
+        // content/common/resource_messages.cc
+        NOTREACHED();
+        return false;
+
+      case ResourceRequestBodyImpl::Element::TYPE_BYTES:
+      case ResourceRequestBodyImpl::Element::TYPE_BYTES_DESCRIPTION:
+        // Data is self-contained within |body| - no need to check access.
+        break;
+
+      case ResourceRequestBodyImpl::Element::TYPE_BLOB:
+        // No need to validate - the unguessability of the uuid of the blob is a
+        // sufficient defense against access from an unrelated renderer.
+        break;
+
+      case ResourceRequestBodyImpl::Element::TYPE_UNKNOWN:
+      default:
+        // Fail safe - deny access.
+        NOTREACHED();
+        return false;
+    }
+  }
+  return true;
+}
+
+}  // namespace
+
+bool CanReadRequestBody(SiteInstance* site_instance,
+                        const scoped_refptr<ResourceRequestBodyImpl>& body) {
+  int child_id = site_instance->GetProcess()->GetID();
+
+  StoragePartition* storage_partition = BrowserContext::GetStoragePartition(
+      site_instance->GetBrowserContext(), site_instance);
+  const storage::FileSystemContext* file_system_context =
+      storage_partition->GetFileSystemContext();
+
+  return CanReadRequestBody(child_id, file_system_context, body);
+}
+
+}  // namespace content