Merge pull request #8900 from electron/handle-buffer-instances-in-sandbox

Handle Buffer deserialization in sandboxed renderers

atom/common/native_mate_converters/v8_value_converter.cc

@@ -129,6 +129,7 @@ class V8ValueConverter::ScopedUniquenessGuard {
 V8ValueConverter::V8ValueConverter()
     : reg_exp_allowed_(false),
       function_allowed_(false),
+      disable_node_(false),
       strip_null_from_objects_(false) {}
 
 void V8ValueConverter::SetRegExpAllowed(bool val) {
@@ -143,6 +144,10 @@ void V8ValueConverter::SetStripNullFromObjects(bool val) {
   strip_null_from_objects_ = val;
 }
 
+void V8ValueConverter::SetDisableNode(bool val) {
+  disable_node_ = val;
+}
+
 v8::Local<v8::Value> V8ValueConverter::ToV8Value(
     const base::Value* value, v8::Local<v8::Context> context) const {
   v8::Context::Scope context_scope(context);
@@ -249,9 +254,49 @@ v8::Local<v8::Value> V8ValueConverter::ToV8Object(
 
 v8::Local<v8::Value> V8ValueConverter::ToArrayBuffer(
     v8::Isolate* isolate, const base::BinaryValue* value) const {
-  return node::Buffer::Copy(isolate,
+  const char* data = value->GetBuffer();
-                            value->GetBuffer(),
+  size_t length = value->GetSize();
-                            value->GetSize()).ToLocalChecked();
+
+  if (!disable_node_) {
+    return node::Buffer::Copy(isolate, data, length).ToLocalChecked();
+  }
+
+  if (length > node::Buffer::kMaxLength) {
+    return v8::Local<v8::Object>();
+  }
+  auto context = isolate->GetCurrentContext();
+  auto array_buffer = v8::ArrayBuffer::New(isolate, length);
+  memcpy(array_buffer->GetContents().Data(), data, length);
+  // From this point, if something goes wrong(can't find Buffer class for
+  // example) we'll simply return a Uint8Array based on the created ArrayBuffer.
+  // This can happen if no preload script was specified to the renderer.
+  mate::Dictionary global(isolate, context->Global());
+  v8::Local<v8::Value> buffer_value;
+
+  // Get the Buffer class stored as a hidden value in the global object. We'll
+  // use it return a browserified Buffer.
+  if (!global.GetHidden("Buffer", &buffer_value) ||
+      !buffer_value->IsFunction()) {
+    return v8::Uint8Array::New(array_buffer, 0, length);
+  }
+
+  mate::Dictionary buffer_class(isolate, buffer_value->ToObject());
+  v8::Local<v8::Value> from_value;
+  if (!buffer_class.Get("from", &from_value) ||
+      !from_value->IsFunction()) {
+    return v8::Uint8Array::New(array_buffer, 0, length);
+  }
+
+  v8::Local<v8::Value> args[] = {
+    array_buffer
+  };
+  auto func = v8::Local<v8::Function>::Cast(from_value);
+  auto result = func->Call(context, v8::Null(isolate), 1, args);
+  if (!result.IsEmpty()) {
+    return result.ToLocalChecked();
+  }
+
+  return v8::Uint8Array::New(array_buffer, 0, length);
 }
 
 base::Value* V8ValueConverter::FromV8ValueImpl(

atom/common/native_mate_converters/v8_value_converter.h

@@ -25,6 +25,7 @@ class V8ValueConverter {
   void SetRegExpAllowed(bool val);
   void SetFunctionAllowed(bool val);
   void SetStripNullFromObjects(bool val);
+  void SetDisableNode(bool val);
   v8::Local<v8::Value> ToV8Value(const base::Value* value,
                                  v8::Local<v8::Context> context) const;
   base::Value* FromV8Value(v8::Local<v8::Value> value,
@@ -64,6 +65,13 @@ class V8ValueConverter {
   // If true, we will convert Function JavaScript objects to dictionaries.
   bool function_allowed_;
 
+  // If true, will not use node::Buffer::Copy to deserialize byte arrays.
+  // node::Buffer::Copy depends on a working node.js environment, and this is
+  // not desirable in sandboxed renderers. That means Buffer instances sent from
+  // browser process will be deserialized as browserify-based Buffer(which are
+  // wrappers around Uint8Array).
+  bool disable_node_;
+
   // If true, undefined and null values are ignored when converting v8 objects
   // into Values.
   bool strip_null_from_objects_;

atom/renderer/atom_sandboxed_renderer_client.cc

@@ -10,6 +10,7 @@
 
 #include "atom/common/api/api_messages.h"
 #include "atom/common/native_mate_converters/string16_converter.h"
+#include "atom/common/native_mate_converters/v8_value_converter.h"
 #include "atom/common/native_mate_converters/value_converter.h"
 #include "atom/common/node_includes.h"
 #include "atom/common/options_switches.h"
@@ -135,7 +136,9 @@ class AtomSandboxedRenderViewObserver : public AtomRenderViewObserver {
   AtomSandboxedRenderViewObserver(content::RenderView* render_view,
                                   AtomSandboxedRendererClient* renderer_client)
     : AtomRenderViewObserver(render_view, nullptr),
+    v8_converter_(new atom::V8ValueConverter),
     renderer_client_(renderer_client) {
+      v8_converter_->SetDisableNode(true);
     }
 
  protected:
@@ -151,7 +154,7 @@ class AtomSandboxedRenderViewObserver : public AtomRenderViewObserver {
     v8::Context::Scope context_scope(context);
     v8::Local<v8::Value> argv[] = {
       mate::ConvertToV8(isolate, channel),
-      mate::ConvertToV8(isolate, args)
+      v8_converter_->ToV8Value(&args, context)
     };
     renderer_client_->InvokeIpcCallback(
         context,
@@ -160,6 +163,7 @@ class AtomSandboxedRenderViewObserver : public AtomRenderViewObserver {
   }
 
  private:
+  std::unique_ptr<atom::V8ValueConverter> v8_converter_;
   AtomSandboxedRendererClient* renderer_client_;
   DISALLOW_COPY_AND_ASSIGN(AtomSandboxedRenderViewObserver);
 };

lib/sandboxed_renderer/init.js

@@ -5,6 +5,9 @@ const events = require('events')
 process.atomBinding = require('../common/atom-binding-setup')(binding.get, 'renderer')
 
 const v8Util = process.atomBinding('v8_util')
+// Expose browserify Buffer as a hidden value. This is used by C++ code to
+// deserialize Buffer instances sent from browser process.
+v8Util.setHiddenValue(global, 'Buffer', Buffer)
 // The `lib/renderer/api/ipc-renderer.js` module looks for the ipc object in the
 // "ipc" hidden value
 v8Util.setHiddenValue(global, 'ipc', new events.EventEmitter())