From da07709c74b7a695fa2155718a9d9b193ffd169a Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Fri, 2 Aug 2013 12:52:43 -0700 Subject: [PATCH] smalloc: fix assertion fails/segfault * Numeric values passed to alloc were converted to int32, not uint32 before the range check, which allows wrap around on ToUint32. This would cause massive malloc calls and v8 fatal errors. * dispose would not check if value was an Object, causing segfault if a Primitive was passed. * kMaxLength was not enumerable. --- lib/smalloc.js | 16 ++++++++++++---- test/simple/test-smalloc.js | 23 +++++++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/lib/smalloc.js b/lib/smalloc.js index cba96ac..fa1bf23 100644 --- a/lib/smalloc.js +++ b/lib/smalloc.js @@ -30,25 +30,33 @@ exports.dispose = dispose; // don't allow kMaxLength to accidentally be overwritten. it's a lot less // apparent when a primitive is accidentally changed. Object.defineProperty(exports, 'kMaxLength', { - writable: false, - value: kMaxLength + enumerable: true, + value: kMaxLength, + writable: false }); function alloc(n, obj) { - n = ~~n; + n = n >>> 0; + if (util.isUndefined(obj)) + obj = {}; + else if (util.isPrimitive(obj)) + throw new TypeError('obj must be an Object'); if (n > kMaxLength) throw new RangeError('n > kMaxLength'); if (util.isArray(obj)) throw new TypeError('Arrays are not supported'); - return smalloc.alloc(util.isUndefined(obj) ? {} : obj, n); + return smalloc.alloc(obj, n); } function dispose(obj) { + if (util.isPrimitive(obj)) + throw new TypeError('obj must be an Object'); if (util.isBuffer(obj)) throw new TypeError('obj cannot be a Buffer'); + smalloc.dispose(obj); } diff --git a/test/simple/test-smalloc.js b/test/simple/test-smalloc.js index 3729adb..ceba0a4 100644 --- a/test/simple/test-smalloc.js +++ b/test/simple/test-smalloc.js @@ -107,6 +107,12 @@ assert.throws(function() { }, RangeError); +// properly convert to uint32 before checking overflow +assert.throws(function() { + alloc(-1); +}, RangeError); + + // no allocating on what's been allocated assert.throws(function() { alloc(1, alloc(1)); @@ -123,6 +129,9 @@ assert.throws(function() { assert.throws(function() { alloc(1, true); }, TypeError); +assert.throws(function() { + alloc(1, null); +}, TypeError); // should not throw allocating to most objects @@ -186,3 +195,17 @@ var b = alloc(5, {}); dispose(b); for (var i = 0; i < 5; i++) assert.equal(b[i], undefined); + + +// verify dispose throws properly + +// only allow object to be passed to dispose +assert.throws(function() { + alloc.dispose(null); +}); + + +// can't dispose a Buffer +assert.throws(function() { + alloc.dispose(new Buffer()); +}); -- 2.7.4