Skip to content

Commit

Permalink
buffer: fix copy() segfault with zero arguments
Browse files Browse the repository at this point in the history
Buffer#copy() immediately does a ToObject() on the first argument before
it checks if it's even an Object. This causes
Object::HasIndexedPropertiesInExternalArrayData() to be run on nothing,
triggering the segfault. Instead run HasInstance() on the args Value.
Which will check if it's actually an Object, before checking if it
contains data.

Fixes: #1519
PR-URL: #1520
Reviewed-by: Evan Lucas <evanlucas@me.com>
  • Loading branch information
trevnorris committed Apr 24, 2015
1 parent 2f6986e commit 5404cbc
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 3 deletions.
6 changes: 3 additions & 3 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -303,11 +303,11 @@ void Base64Slice(const FunctionCallbackInfo<Value>& args) {
void Copy(const FunctionCallbackInfo<Value> &args) {
Environment* env = Environment::GetCurrent(args);

Local<Object> target = args[0]->ToObject(env->isolate());

if (!HasInstance(target))
if (!HasInstance(args[0]))
return env->ThrowTypeError("first arg should be a Buffer");

Local<Object> target = args[0]->ToObject(env->isolate());

ARGS_THIS(args.This())
size_t target_length = target->GetIndexedPropertiesExternalArrayDataLength();
char* target_data = static_cast<char*>(
Expand Down
5 changes: 5 additions & 0 deletions test/parallel/test-buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1179,3 +1179,8 @@ var ps = Buffer.poolSize;
Buffer.poolSize = 0;
assert.equal(Buffer(1).parent, undefined);
Buffer.poolSize = ps;

// Test Buffer.copy() segfault
assert.throws(function() {
Buffer(10).copy();
});

0 comments on commit 5404cbc

Please sign in to comment.