Ensure that privates are private.

extensions/renderer/utils_native_handler.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 "extensions/renderer/utils_native_handler.h"
 
+#include <algorithm>
+
 #include "base/macros.h"
+#include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "extensions/renderer/script_context.h"
 #include "third_party/WebKit/public/web/WebSerializedScriptValue.h"
 
 namespace extensions {
 
+namespace {
+
+// Returns whether |name| can safely be used as an identifier in JavaScript code
+// without side effects. This method allows syntax errors, but not execution of
+// arbitrary JavaScript code.
+bool IsAllowedFunctionName(const std::string& name) {
+  return std::find_if_not(name.begin(), name.end(), [](char c) {
+           return base::IsAsciiAlpha(c);
+         }) == name.end();
+}
+
+}  // namespace
+
 UtilsNativeHandler::UtilsNativeHandler(ScriptContext* context)
     : ObjectBackedNativeHandler(context) {
   RouteFunction("createClassWrapper",
                 base::Bind(&UtilsNativeHandler::CreateClassWrapper,
                            base::Unretained(this)));
   RouteFunction(
       "deepCopy",
       base::Bind(&UtilsNativeHandler::DeepCopy, base::Unretained(this)));
 }
 
 UtilsNativeHandler::~UtilsNativeHandler() {}
 
 void UtilsNativeHandler::CreateClassWrapper(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
   CHECK_EQ(3, args.Length());
   CHECK(args[0]->IsString());
   std::string name = *v8::String::Utf8Value(args[0]);
+  CHECK(IsAllowedFunctionName(name));
   CHECK(args[1]->IsObject());
   v8::Local<v8::Object> cls = args[1].As<v8::Object>();
   CHECK(args[2]->IsObject() || args[2]->IsUndefined());
   v8::Local<v8::Value> superclass = args[2];
 
   v8::HandleScope handle_scope(GetIsolate());
   // TODO(fsamuel): Consider moving the source wrapping to ModuleSystem.
   v8::Local<v8::String> source = v8::String::NewFromUtf8(
       GetIsolate(),
       base::StringPrintf(
           "(function($Object, $Function, privates, cls, superclass) {"
           "'use strict';\n"
           "  function %s() {\n"

[Devlin, 2016/04/20 17:42:07]

Wow.  I don't think I've ever looked closely at this code.  Executing from JS -
crazy.

I'm also a little confused on why exactly this needs to be in C++ - utils has
privates already, so what's the benefit in having this in a native handler at
all?  Could we just have utils.expose construct it's own function?

[robwu, 2016/04/21 08:48:11]

This exists to generate meaningful stack traces, see https://crbug.com/345429

[robwu, 2016/04/21 17:29:35]

With that snippet, you'll be in the same situation again as
https://crbug.com/345429, except then the functions are not called
"publicClass", but anonymous.

What do you think of something like this? Three lines extra per caller in
exchange for no natives.

function Port(params) {
  privates(utils).constructPrivate(this, arguments);
}

utils.expose(Port, PortImpl, {__proto__:null, ...func & prop...});

// utils.js
function expose(publicClass, cls, exposed) {
  logging.CHECK(typeof publicClass === 'function');
  if (typeof exposed.superclass === 'function') {
    publicClass.prototype = $Object.create(exposed.superclass.prototype);
  }
  // Set up functions/properties/readonly as currently implemented in expose.
  ...

  privates(publicClass).constructPrivate = function(self, args) {
    var privateObj = $Object.create(cls.prototype);
    $Function.apply(cls, privateObj, args);
    privateObj.wrapper = self;
    privates(self).impl = privateObj;
  };
}

[robwu, 2016/04/21 17:33:36]

privates(utils).constructPrivate should be privates(Port).constructPrivate. Or
even abstracted away to utils and then utils.privateCostructor(port)(this,
arguments), or some other signature. WDYT?

           "    var privateObj = $Object.create(cls.prototype);\n"
           "    $Function.apply(cls, privateObj, arguments);\n"
           "    privateObj.wrapper = this;\n"
           "    privates(this).impl = privateObj;\n"
           "  };\n"
           "  if (superclass) {\n"
           "    %s.prototype = Object.create(superclass.prototype);\n"
           "  }\n"
           "  return %s;\n"
           "})",
@@ skipping 31 matching lines @@
 }
 
 void UtilsNativeHandler::DeepCopy(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
   CHECK_EQ(1, args.Length());
   args.GetReturnValue().Set(
       blink::WebSerializedScriptValue::serialize(args[0]).deserialize());
 }
 
 }  // namespace extensions