buffer: throw from constructor if length > kMaxLength

Throw, don't abort. `new Buffer(0x3fffffff + 1)` used to bring down the process with the following error message: FATAL ERROR: v8::Object::SetIndexedPropertiesToExternalArrayData() length exceeds max acceptable value Fixes #2280.
nodejs · Mar 9, 2012 · 8c02f9b · 8c02f9b
1 parent 2589d55
commit 8c02f9b
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 7 deletions.
diff --git a/src/node_buffer.cc b/src/node_buffer.cc
@@ -171,13 +171,14 @@ Handle<Value> Buffer::New(const Arguments &args) {
 
   HandleScope scope;
 
-  if (args[0]->IsInt32()) {
-    // var buffer = new Buffer(1024);
-    size_t length = args[0]->Uint32Value();
-    new Buffer(args.This(), length);
-  } else {
-    return ThrowException(Exception::TypeError(String::New("Bad argument")));
+  if (!args[0]->IsUint32()) return ThrowTypeError("Bad argument");
+
+  size_t length = args[0]->Uint32Value();
+  if (length > Buffer::kMaxLength) {
+    return ThrowRangeError("length > kMaxLength");
   }
+  new Buffer(args.This(), length);
+
   return args.This();
 }
 

diff --git a/src/node_buffer.h b/src/node_buffer.h
@@ -65,6 +65,9 @@ namespace node {
 
 class NODE_EXTERN Buffer: public ObjectWrap {
  public:
+  // mirrors deps/v8/src/objects.h
+  static const int kMaxLength = 0x3fffffff;
+
   static v8::Persistent<v8::FunctionTemplate> constructor_template;
 
   static bool HasInstance(v8::Handle<v8::Value> val);

diff --git a/src/v8_typed_array.cc b/src/v8_typed_array.cc
@@ -91,6 +91,10 @@ class ArrayBuffer {
     }
 
     size_t num_bytes = args[0]->Uint32Value();
+    if (num_bytes > node::Buffer::kMaxLength) {
+      return ThrowRangeError("length > kMaxLength");
+    }
+
     void* buf = calloc(num_bytes, 1);
     if (!buf)
       return ThrowError("Unable to allocate ArrayBuffer.");
@@ -224,6 +228,7 @@ class TypedArray {
           v8::Integer::NewFromUnsigned(length * TBytes)};
       buffer = ArrayBuffer::GetTemplate()->
                  GetFunction()->NewInstance(1, argv);
+      if (buffer.IsEmpty()) return v8::Undefined(); // constructor failed
 
       void* buf = buffer->GetPointerFromInternalField(0);
       args.This()->SetIndexedPropertiesToExternalArrayData(
@@ -252,8 +257,9 @@ class TypedArray {
 
       buffer = ArrayBuffer::GetTemplate()->
                  GetFunction()->NewInstance(1, argv);
-      void* buf = buffer->GetPointerFromInternalField(0);
+      if (buffer.IsEmpty()) return v8::Undefined(); // constructor failed
 
+      void* buf = buffer->GetPointerFromInternalField(0);
       args.This()->SetIndexedPropertiesToExternalArrayData(
           buf, TEAType, length);
       // TODO(deanm): check for failure.

diff --git a/test/pummel/test-buffer-big.js b/test/pummel/test-buffer-big.js
@@ -0,0 +1,29 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var common = require('../common');
+var assert = require('assert');
+
+// The tests below should throw an error, not abort the process...
+assert.throws(function() { new Buffer(0x3fffffff + 1) }, RangeError);
+assert.throws(function() { new Int8Array(0x3fffffff + 1) }, RangeError);
+assert.throws(function() { new ArrayBuffer(0x3fffffff + 1) }, RangeError);
+assert.throws(function() { new Float64Array(0x7ffffff + 1) }, RangeError);