From 75b803b1c81ed9fa5513cbff550232b4fb915e7b Mon Sep 17 00:00:00 2001
From: "rdevlin.cronin" <rdevlin.cronin@chromium.org>
Date: Tue, 1 Mar 2016 16:13:47 -0800
Subject: [Extensions] Harden against bindings interception

There's more we can do but this is a start.

BUG=590275
BUG=590118

Review URL: https://codereview.chromium.org/1748943002

Cr-Commit-Position: refs/heads/master@{#378621}
---
 extensions/renderer/module_system.cc |  4 ++--
 extensions/renderer/v8_helpers.h     | 43 ++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 2 deletions(-)

(limited to 'extensions/renderer')

diff --git a/extensions/renderer/module_system.cc b/extensions/renderer/module_system.cc
index ef96d73..f027413 100644
--- a/extensions/renderer/module_system.cc
+++ b/extensions/renderer/module_system.cc
@@ -251,12 +251,12 @@ v8::Local<v8::Value> ModuleSystem::RequireForJsInner(
 
   v8::Local<v8::Object> modules(v8::Local<v8::Object>::Cast(modules_value));
   v8::Local<v8::Value> exports;
-  if (!GetProperty(v8_context, modules, module_name, &exports) ||
+  if (!GetPrivateProperty(v8_context, modules, module_name, &exports) ||
       !exports->IsUndefined())
     return handle_scope.Escape(exports);
 
   exports = LoadModule(*v8::String::Utf8Value(module_name));
-  SetProperty(v8_context, modules, module_name, exports);
+  SetPrivateProperty(v8_context, modules, module_name, exports);
   return handle_scope.Escape(exports);
 }
 
diff --git a/extensions/renderer/v8_helpers.h b/extensions/renderer/v8_helpers.h
index 2a6fa9c..3017772 100644
--- a/extensions/renderer/v8_helpers.h
+++ b/extensions/renderer/v8_helpers.h
@@ -60,6 +60,9 @@ inline bool IsEmptyOrUndefied(v8::Local<v8::Value> value) {
 
 // SetProperty() family wraps V8::Object::DefineOwnProperty().
 // Returns true on success.
+// NOTE: Think about whether you want this or SetPrivateProperty() below.
+// TODO(devlin): Sort through more of the callers of this and see if we can
+// convert more to be private.
 inline bool SetProperty(v8::Local<v8::Context> context,
                         v8::Local<v8::Object> object,
                         v8::Local<v8::String> key,
@@ -84,8 +87,29 @@ inline bool SetProperty(v8::Local<v8::Context> context,
   return SetProperty(context, object, base::UintToString(index).c_str(), value);
 }
 
+// Wraps v8::Object::SetPrivate(). When possible, prefer this to SetProperty().
+inline bool SetPrivateProperty(v8::Local<v8::Context> context,
+                               v8::Local<v8::Object> object,
+                               v8::Local<v8::String> key,
+                               v8::Local<v8::Value> value) {
+  return IsTrue(object->SetPrivate(
+      context, v8::Private::ForApi(context->GetIsolate(), key), value));
+}
+
+inline bool SetPrivateProperty(v8::Local<v8::Context> context,
+                               v8::Local<v8::Object> object,
+                               const char* key,
+                               v8::Local<v8::Value> value) {
+  v8::Local<v8::String> v8_key;
+  return ToV8String(context->GetIsolate(), key, &v8_key) &&
+         IsTrue(object->SetPrivate(
+             context, v8::Private::ForApi(context->GetIsolate(), v8_key),
+             value));
+}
+
 // GetProperty() family calls V8::Object::Get() and extracts a value from
 // returned MaybeLocal. Returns true on success.
+// NOTE: Think about whether you want this or GetPrivateProperty() below.
 template <typename Key>
 inline bool GetProperty(v8::Local<v8::Context> context,
                         v8::Local<v8::Object> object,
@@ -104,6 +128,25 @@ inline bool GetProperty(v8::Local<v8::Context> context,
   return GetProperty(context, object, v8_key, out);
 }
 
+// Wraps v8::Object::GetPrivate(). When possible, prefer this to GetProperty().
+inline bool GetPrivateProperty(v8::Local<v8::Context> context,
+                               v8::Local<v8::Object> object,
+                               v8::Local<v8::String> key,
+                               v8::Local<v8::Value>* out) {
+  return object
+      ->GetPrivate(context, v8::Private::ForApi(context->GetIsolate(), key))
+      .ToLocal(out);
+}
+
+inline bool GetPrivateProperty(v8::Local<v8::Context> context,
+                               v8::Local<v8::Object> object,
+                               const char* key,
+                               v8::Local<v8::Value>* out) {
+  v8::Local<v8::String> v8_key;
+  return ToV8String(context->GetIsolate(), key, &v8_key) &&
+         GetPrivateProperty(context, object, v8_key, out);
+}
+
 // GetPropertyUnsafe() family wraps v8::Object::Get(). They crash when an
 // exception is thrown.
 inline v8::Local<v8::Value> GetPropertyUnsafe(v8::Local<v8::Context> context,
-- 
cgit v1.1