From ca408dcb586e3273c824d1eecc87e80da9cf2aef Mon Sep 17 00:00:00 2001 From: Brendan Ashworth Date: Mon, 8 Jun 2015 23:21:46 -0700 Subject: [PATCH] buffer: prevent failed assertions This commit adds assert() calls in JS, which prevents non-buffer instances from triggering failed C++ assertions (killing the process). They will now die with an AssertionError. Buffer.prototype.copy had to be wrapped in JS to allow for the assertion, with a modification to the arguments passed. Fixes: https://github.com/nodejs/io.js/issues/1485 --- lib/buffer.js | 42 ++++++++++++++++++++++++++++++++---- src/node_buffer.cc | 16 ++++++-------- test/parallel/test-buffer.js | 42 ++++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 14 deletions(-) diff --git a/lib/buffer.js b/lib/buffer.js index ccd899da5f32ae..fe1749b7c891f0 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -3,6 +3,7 @@ const binding = process.binding('buffer'); const smalloc = process.binding('smalloc'); const util = require('util'); +const assert = require('assert'); const internalUtil = require('internal/util'); const alloc = smalloc.alloc; const truncate = smalloc.truncate; @@ -335,6 +336,14 @@ Buffer.byteLength = byteLength; // toString(encoding, start=0, end=buffer.length) Buffer.prototype.toString = function(encoding, start, end) { + // .toString() can be called automatically on non-Buffers. + if (!(this instanceof Buffer)) { + if (arguments.length === 0) + return Object.prototype.toString.call(this); + else + assert(this instanceof Buffer); + } + var loweredCase = false; start = start >>> 0; @@ -380,8 +389,7 @@ Buffer.prototype.toString = function(encoding, start, end) { Buffer.prototype.equals = function equals(b) { - if (!(b instanceof Buffer)) - throw new TypeError('Argument must be a Buffer'); + assert(this instanceof Buffer && b instanceof Buffer); if (this === b) return true; @@ -404,8 +412,7 @@ Buffer.prototype.inspect = function inspect() { Buffer.prototype.compare = function compare(b) { - if (!(b instanceof Buffer)) - throw new TypeError('Argument must be a Buffer'); + assert(this instanceof Buffer && b instanceof Buffer); if (this === b) return 0; @@ -415,6 +422,8 @@ Buffer.prototype.compare = function compare(b) { Buffer.prototype.indexOf = function indexOf(val, byteOffset) { + assert(this instanceof Buffer); + if (byteOffset > 0x7fffffff) byteOffset = 0x7fffffff; else if (byteOffset < -0x80000000) @@ -433,6 +442,8 @@ Buffer.prototype.indexOf = function indexOf(val, byteOffset) { Buffer.prototype.fill = function fill(val, start, end) { + assert(this instanceof Buffer); + start = start >> 0; end = (end === undefined) ? this.length : end >> 0; @@ -455,6 +466,13 @@ Buffer.prototype.fill = function fill(val, start, end) { }; +Buffer.prototype.copy = function copy(target, start, sourceStart, sourceEnd) { + assert(this instanceof Buffer && target instanceof Buffer); + + return binding.copy(this, target, start, sourceStart, sourceEnd); +}; + + // XXX remove in v0.13 Buffer.prototype.get = util.deprecate(function get(offset) { offset = ~~offset; @@ -779,6 +797,8 @@ Buffer.prototype.readInt32BE = function(offset, noAssert) { Buffer.prototype.readFloatLE = function readFloatLE(offset, noAssert) { + assert(this instanceof Buffer); + offset = offset >>> 0; if (!noAssert) checkOffset(offset, 4, this.length); @@ -787,6 +807,8 @@ Buffer.prototype.readFloatLE = function readFloatLE(offset, noAssert) { Buffer.prototype.readFloatBE = function readFloatBE(offset, noAssert) { + assert(this instanceof Buffer); + offset = offset >>> 0; if (!noAssert) checkOffset(offset, 4, this.length); @@ -795,6 +817,8 @@ Buffer.prototype.readFloatBE = function readFloatBE(offset, noAssert) { Buffer.prototype.readDoubleLE = function readDoubleLE(offset, noAssert) { + assert(this instanceof Buffer); + offset = offset >>> 0; if (!noAssert) checkOffset(offset, 8, this.length); @@ -803,6 +827,8 @@ Buffer.prototype.readDoubleLE = function readDoubleLE(offset, noAssert) { Buffer.prototype.readDoubleBE = function readDoubleBE(offset, noAssert) { + assert(this instanceof Buffer); + offset = offset >>> 0; if (!noAssert) checkOffset(offset, 8, this.length); @@ -1025,6 +1051,8 @@ function checkFloat(buffer, value, offset, ext) { Buffer.prototype.writeFloatLE = function writeFloatLE(val, offset, noAssert) { + assert(this instanceof Buffer); + val = +val; offset = offset >>> 0; if (!noAssert) @@ -1035,6 +1063,8 @@ Buffer.prototype.writeFloatLE = function writeFloatLE(val, offset, noAssert) { Buffer.prototype.writeFloatBE = function writeFloatBE(val, offset, noAssert) { + assert(this instanceof Buffer); + val = +val; offset = offset >>> 0; if (!noAssert) @@ -1045,6 +1075,8 @@ Buffer.prototype.writeFloatBE = function writeFloatBE(val, offset, noAssert) { Buffer.prototype.writeDoubleLE = function writeDoubleLE(val, offset, noAssert) { + assert(this instanceof Buffer); + val = +val; offset = offset >>> 0; if (!noAssert) @@ -1055,6 +1087,8 @@ Buffer.prototype.writeDoubleLE = function writeDoubleLE(val, offset, noAssert) { Buffer.prototype.writeDoubleBE = function writeDoubleBE(val, offset, noAssert) { + assert(this instanceof Buffer); + val = +val; offset = offset >>> 0; if (!noAssert) diff --git a/src/node_buffer.cc b/src/node_buffer.cc index fca08599e50feb..650729d45f8653 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -303,12 +303,9 @@ void Base64Slice(const FunctionCallbackInfo& args) { void Copy(const FunctionCallbackInfo &args) { Environment* env = Environment::GetCurrent(args); - if (!HasInstance(args[0])) - return env->ThrowTypeError("first arg should be a Buffer"); + Local target = args[1]->ToObject(env->isolate()); - Local target = args[0]->ToObject(env->isolate()); - - ARGS_THIS(args.This()) + ARGS_THIS(args[0].As()); size_t target_length = target->GetIndexedPropertiesExternalArrayDataLength(); char* target_data = static_cast( target->GetIndexedPropertiesExternalArrayData()); @@ -316,9 +313,9 @@ void Copy(const FunctionCallbackInfo &args) { size_t source_start; size_t source_end; - CHECK_NOT_OOB(ParseArrayIndex(args[1], 0, &target_start)); - CHECK_NOT_OOB(ParseArrayIndex(args[2], 0, &source_start)); - CHECK_NOT_OOB(ParseArrayIndex(args[3], obj_length, &source_end)); + CHECK_NOT_OOB(ParseArrayIndex(args[2], 0, &target_start)); + CHECK_NOT_OOB(ParseArrayIndex(args[3], 0, &source_start)); + CHECK_NOT_OOB(ParseArrayIndex(args[4], obj_length, &source_end)); // Copy 0 bytes; we're done if (target_start >= target_length || source_start >= source_end) @@ -723,8 +720,6 @@ void SetupBufferJS(const FunctionCallbackInfo& args) { env->SetMethod(proto, "ucs2Write", Ucs2Write); env->SetMethod(proto, "utf8Write", Utf8Write); - env->SetMethod(proto, "copy", Copy); - // for backwards compatibility proto->ForceSet(env->offset_string(), Uint32::New(env->isolate(), 0), @@ -742,6 +737,7 @@ void Initialize(Handle target, env->SetMethod(target, "byteLengthUtf8", ByteLengthUtf8); env->SetMethod(target, "compare", Compare); env->SetMethod(target, "fill", Fill); + env->SetMethod(target, "copy", Copy); env->SetMethod(target, "indexOfBuffer", IndexOfBuffer); env->SetMethod(target, "indexOfNumber", IndexOfNumber); env->SetMethod(target, "indexOfString", IndexOfString); diff --git a/test/parallel/test-buffer.js b/test/parallel/test-buffer.js index 1d02148734e38a..c189e77344f133 100644 --- a/test/parallel/test-buffer.js +++ b/test/parallel/test-buffer.js @@ -1172,3 +1172,45 @@ Buffer.poolSize = ps; assert.throws(function() { Buffer(10).copy(); }); + + +// https://github.com/nodejs/io.js/issues/1485 +// C++ bindings fail assertions (die) when called on non-buffers. +assert.equal(Buffer.prototype.toString(), '[object Object]'); + +[null, undefined, {}, 0].forEach(function(v) { + assert.throws(function() { + Buffer.prototype.write.call(v); + }); + + assert.throws(function() { + Buffer.prototype.copy.call(v, new Buffer(0)); + }); + assert.throws(function() { + (new Buffer(0)).copy(v); + }); + + assert.throws(function() { + Buffer.prototype.equals.call(v, new Buffer('hi')); + }); + + assert.throws(function() { + Buffer.prototype.compare.call(v, new Buffer('hi')); + }); + + assert.throws(function() { + Buffer.prototype.fill.call(v); + }); + + assert.throws(function() { + Buffer.prototype.indexOf.call(v, 0); + }); + + assert.throws(function() { + Buffer.prototype.readDoubleLE.call(v, 0, false); + }); + + assert.throws(function() { + Buffer.prototype.writeDoubleLE.call(v, 0, 0, false); + }); +});