Replace v8::Object::Set with CreateDataProperty for security reasons.

chrome/renderer/extensions/automation_internal_custom_bindings.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 "chrome/renderer/extensions/automation_internal_custom_bindings.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <memory>
 
 #include "base/bind.h"
 #include "base/macros.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/values.h"
 #include "chrome/common/extensions/api/automation_api_constants.h"
 #include "chrome/common/extensions/chrome_extension_messages.h"
 #include "chrome/common/extensions/manifest_handlers/automation.h"
 #include "content/public/renderer/render_frame.h"
 #include "content/public/renderer/render_thread.h"
 #include "content/public/renderer/render_view.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/manifest.h"
 #include "extensions/common/manifest_handlers/background_info.h"
 #include "extensions/renderer/extension_bindings_system.h"
 #include "extensions/renderer/script_context.h"
+#include "gin/converter.h"
 #include "ipc/message_filter.h"
 #include "ui/accessibility/ax_enums.h"
 #include "ui/accessibility/ax_node.h"
 #include "ui/gfx/geometry/rect_conversions.h"
 
 namespace extensions {
 
 namespace {
 
 void ThrowInvalidArgumentsException(
     AutomationInternalCustomBindings* automation_bindings) {
   v8::Isolate* isolate = automation_bindings->GetIsolate();
   automation_bindings->GetIsolate()->ThrowException(
       v8::String::NewFromUtf8(
           isolate,
           "Invalid arguments to AutomationInternalCustomBindings function",
           v8::NewStringType::kNormal)
           .ToLocalChecked());
 
   LOG(FATAL) << "Invalid arguments to AutomationInternalCustomBindings function"
              << automation_bindings->context()->GetStackTraceAsString();
 }
 
-v8::Local<v8::Value> CreateV8String(v8::Isolate* isolate, const char* str) {
-  return v8::String::NewFromUtf8(isolate, str, v8::String::kNormalString,
-                                 strlen(str));
+v8::Local<v8::String> CreateV8String(v8::Isolate* isolate,
+                                     base::StringPiece str) {
+  return gin::StringToSymbol(isolate, str);
 }
 
-v8::Local<v8::Value> CreateV8String(v8::Isolate* isolate,
-                                    const std::string& str) {
-  return v8::String::NewFromUtf8(isolate, str.c_str(),
-                                 v8::String::kNormalString, str.length());
+// Note: when building up an object to return from one of the
+// automation API bindings like a rect ({left: 0, top: 0, ...}) or
+// something like that, we should use this function instead of
+// v8::Object::Set, because a malicious extension author could use
+// Object.defineProperty to override a setter and trigger all sorts of
+// things to happen in the middle of one of our functions below.
+//
+// This is only safe when we're creating the object to return and
+// we're setting properties on it before it's been exposed to
+// untrusted scripts.
+void SafeSetV8Property(v8::Isolate* isolate,
+                       v8::Local<v8::Object> object,
+                       base::StringPiece key,
+                       v8::Local<v8::Value> value) {
+  v8::Maybe<bool> maybe = object->CreateDataProperty(
+      isolate->GetCurrentContext(), CreateV8String(isolate, key), value);
+
+  // There's no legit reason CreateDataProperty should fail.
+  CHECK(maybe.IsJust() && maybe.FromJust());
 }
 
 v8::Local<v8::Object> RectToV8Object(v8::Isolate* isolate,
                                      const gfx::Rect& rect) {
   v8::Local<v8::Object> result(v8::Object::New(isolate));
-  result->Set(CreateV8String(isolate, "left"),
-              v8::Integer::New(isolate, rect.x()));
-  result->Set(CreateV8String(isolate, "top"),
-              v8::Integer::New(isolate, rect.y()));
-  result->Set(CreateV8String(isolate, "width"),
-              v8::Integer::New(isolate, rect.width()));
-  result->Set(CreateV8String(isolate, "height"),
-              v8::Integer::New(isolate, rect.height()));
+  SafeSetV8Property(isolate, result, "left",
+                    v8::Integer::New(isolate, rect.x()));
+  SafeSetV8Property(isolate, result, "top",
+                    v8::Integer::New(isolate, rect.y()));
+  SafeSetV8Property(isolate, result, "width",
+                    v8::Integer::New(isolate, rect.width()));
+  SafeSetV8Property(isolate, result, "height",
+                    v8::Integer::New(isolate, rect.height()));
   return result;
 }
 
 // Compute the bounding box of a node, fixing nodes with empty bounds by
 // unioning the bounds of their children.
 static gfx::RectF ComputeLocalNodeBounds(TreeCache* cache, ui::AXNode* node) {
   gfx::RectF bounds = node->data().location;
   if (bounds.width() > 0 && bounds.height() > 0)
     return bounds;
 
@@ skipping 856 matching lines @@
   if (!cache)
     return;
 
   TreeCache* focused_tree_cache = nullptr;
   ui::AXNode* focused_node = nullptr;
   if (!GetFocusInternal(cache, &focused_tree_cache, &focused_node))
     return;
 
   v8::Isolate* isolate = GetIsolate();
   v8::Local<v8::Object> result(v8::Object::New(isolate));
-  result->Set(CreateV8String(isolate, "treeId"),
-              v8::Integer::New(isolate, focused_tree_cache->tree_id));
-  result->Set(CreateV8String(isolate, "nodeId"),
-              v8::Integer::New(isolate, focused_node->id()));
+  SafeSetV8Property(isolate, result, "treeId",
+                    v8::Integer::New(isolate, focused_tree_cache->tree_id));
+  SafeSetV8Property(isolate, result, "nodeId",
+                    v8::Integer::New(isolate, focused_node->id()));
   args.GetReturnValue().Set(result);
 }
 
 void AutomationInternalCustomBindings::GetHtmlAttributes(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
   v8::Isolate* isolate = GetIsolate();
   if (args.Length() < 2 || !args[0]->IsNumber() || !args[1]->IsNumber())
     ThrowInvalidArgumentsException(this);
 
   int tree_id = args[0]->Int32Value();
   int node_id = args[1]->Int32Value();
 
   TreeCache* cache = GetTreeCacheFromTreeID(tree_id);
   if (!cache)
     return;
 
   ui::AXNode* node = cache->tree.GetFromId(node_id);
   if (!node)
     return;
 
   v8::Local<v8::Object> dst(v8::Object::New(isolate));
   base::StringPairs src = node->data().html_attributes;
   for (size_t i = 0; i < src.size(); i++) {
     std::string& key = src[i].first;
     std::string& value = src[i].second;
-    dst->Set(CreateV8String(isolate, key), CreateV8String(isolate, value));
+    SafeSetV8Property(isolate, dst, key, CreateV8String(isolate, value));
   }
   args.GetReturnValue().Set(dst);
 }
 
 void AutomationInternalCustomBindings::GetState(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
   v8::Isolate* isolate = GetIsolate();
   if (args.Length() < 2 || !args[0]->IsNumber() || !args[1]->IsNumber())
     ThrowInvalidArgumentsException(this);
 
   int tree_id = args[0]->Int32Value();
   int node_id = args[1]->Int32Value();
 
   TreeCache* cache = GetTreeCacheFromTreeID(tree_id);
   if (!cache)
     return;
 
   ui::AXNode* node = cache->tree.GetFromId(node_id);
   if (!node)
     return;
 
   v8::Local<v8::Object> state(v8::Object::New(isolate));
   uint32_t state_pos = 0, state_shifter = node->data().state;
   while (state_shifter) {
     if (state_shifter & 1) {
       std::string key = ToString(static_cast<ui::AXState>(state_pos));
-      state->Set(CreateV8String(isolate, key), v8::Boolean::New(isolate, true));
+      SafeSetV8Property(isolate, state, key, v8::Boolean::New(isolate, true));
     }
     state_shifter = state_shifter >> 1;
     state_pos++;
   }
 
   TreeCache* top_cache = GetTreeCacheFromTreeID(0);
   if (!top_cache)
     top_cache = cache;
   TreeCache* focused_cache = nullptr;
   ui::AXNode* focused_node = nullptr;
   if (GetFocusInternal(top_cache, &focused_cache, &focused_node)) {
     if (focused_cache == cache && focused_node == node) {
-      state->Set(CreateV8String(isolate, "focused"),
-                 v8::Boolean::New(isolate, true));
+      SafeSetV8Property(isolate, state, "focused",
+                        v8::Boolean::New(isolate, true));
     }
   }
   if (cache->tree.data().focus_id == node->id()) {
-    state->Set(CreateV8String(isolate, "focused"),
-               v8::Boolean::New(isolate, true));
+    SafeSetV8Property(isolate, state, "focused",
+                      v8::Boolean::New(isolate, true));
   }
 
   args.GetReturnValue().Set(state);
 }
 
 void AutomationInternalCustomBindings::UpdateOverallTreeChangeObserverFilter() {
   tree_change_observer_overall_filter_ = 0;
   for (const auto& observer : tree_change_observers_)
     tree_change_observer_overall_filter_ |= 1 << observer.filter;
 }
@@ skipping 379 matching lines @@
     for (auto id : ids)
       nodes->AppendInteger(id);
     args.Append(std::move(nodes));
   }
 
   bindings_system_->DispatchEventInContext("automationInternal.onNodesRemoved",
                                            &args, nullptr, context());
 }
 
 }  // namespace extensions