Skip to content

Commit 8c02f9b

Browse files
committed
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 nodejs#2280.
1 parent 2589d55 commit 8c02f9b

4 files changed

Lines changed: 46 additions & 7 deletions

File tree

src/node_buffer.cc

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -171,13 +171,14 @@ Handle<Value> Buffer::New(const Arguments &args) {
171171

172172
HandleScope scope;
173173

174-
if (args[0]->IsInt32()) {
175-
// var buffer = new Buffer(1024);
176-
size_t length = args[0]->Uint32Value();
177-
new Buffer(args.This(), length);
178-
} else {
179-
return ThrowException(Exception::TypeError(String::New("Bad argument")));
174+
if (!args[0]->IsUint32()) return ThrowTypeError("Bad argument");
175+
176+
size_t length = args[0]->Uint32Value();
177+
if (length > Buffer::kMaxLength) {
178+
return ThrowRangeError("length > kMaxLength");
180179
}
180+
new Buffer(args.This(), length);
181+
181182
return args.This();
182183
}
183184

src/node_buffer.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ namespace node {
6565

6666
class NODE_EXTERN Buffer: public ObjectWrap {
6767
public:
68+
// mirrors deps/v8/src/objects.h
69+
static const int kMaxLength = 0x3fffffff;
70+
6871
static v8::Persistent<v8::FunctionTemplate> constructor_template;
6972

7073
static bool HasInstance(v8::Handle<v8::Value> val);

src/v8_typed_array.cc

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,10 @@ class ArrayBuffer {
9191
}
9292

9393
size_t num_bytes = args[0]->Uint32Value();
94+
if (num_bytes > node::Buffer::kMaxLength) {
95+
return ThrowRangeError("length > kMaxLength");
96+
}
97+
9498
void* buf = calloc(num_bytes, 1);
9599
if (!buf)
96100
return ThrowError("Unable to allocate ArrayBuffer.");
@@ -224,6 +228,7 @@ class TypedArray {
224228
v8::Integer::NewFromUnsigned(length * TBytes)};
225229
buffer = ArrayBuffer::GetTemplate()->
226230
GetFunction()->NewInstance(1, argv);
231+
if (buffer.IsEmpty()) return v8::Undefined(); // constructor failed
227232

228233
void* buf = buffer->GetPointerFromInternalField(0);
229234
args.This()->SetIndexedPropertiesToExternalArrayData(
@@ -252,8 +257,9 @@ class TypedArray {
252257

253258
buffer = ArrayBuffer::GetTemplate()->
254259
GetFunction()->NewInstance(1, argv);
255-
void* buf = buffer->GetPointerFromInternalField(0);
260+
if (buffer.IsEmpty()) return v8::Undefined(); // constructor failed
256261

262+
void* buf = buffer->GetPointerFromInternalField(0);
257263
args.This()->SetIndexedPropertiesToExternalArrayData(
258264
buf, TEAType, length);
259265
// TODO(deanm): check for failure.

test/pummel/test-buffer-big.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Copyright Joyent, Inc. and other Node contributors.
2+
//
3+
// Permission is hereby granted, free of charge, to any person obtaining a
4+
// copy of this software and associated documentation files (the
5+
// "Software"), to deal in the Software without restriction, including
6+
// without limitation the rights to use, copy, modify, merge, publish,
7+
// distribute, sublicense, and/or sell copies of the Software, and to permit
8+
// persons to whom the Software is furnished to do so, subject to the
9+
// following conditions:
10+
//
11+
// The above copyright notice and this permission notice shall be included
12+
// in all copies or substantial portions of the Software.
13+
//
14+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
15+
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
16+
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
17+
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
18+
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
19+
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
20+
// USE OR OTHER DEALINGS IN THE SOFTWARE.
21+
22+
var common = require('../common');
23+
var assert = require('assert');
24+
25+
// The tests below should throw an error, not abort the process...
26+
assert.throws(function() { new Buffer(0x3fffffff + 1) }, RangeError);
27+
assert.throws(function() { new Int8Array(0x3fffffff + 1) }, RangeError);
28+
assert.throws(function() { new ArrayBuffer(0x3fffffff + 1) }, RangeError);
29+
assert.throws(function() { new Float64Array(0x7ffffff + 1) }, RangeError);

0 commit comments

Comments
 (0)