From d964ffeec356167038b4060c867b355d5fea6987 Mon Sep 17 00:00:00 2001 From: Sebastian Van Sande Date: Fri, 3 Feb 2017 16:05:28 +0100 Subject: [PATCH] buffer: check byteLength in readInt(B|L)E MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'byteLength' argument should be required and of type 'number'. It should have a value between 1 and 6. PR-URL: https://github.com/nodejs/node/pull/11146 Fixes: https://github.com/nodejs/node/issues/10515 Reviewed-By: James M Snell Reviewed-By: Joyee Cheung Reviewed-By: Ruben Bridgewater Reviewed-By: Matteo Collina Reviewed-By: Michaël Zasso Reviewed-By: Colin Ihrig --- benchmark/buffers/buffer-read.js | 22 ++++++++++++++++------ doc/api/buffer.md | 5 ++++- lib/buffer.js | 30 ++++++++++++++++++++++++------ test/parallel/test-buffer-read.js | 18 +++++++++++++++++- 4 files changed, 61 insertions(+), 14 deletions(-) diff --git a/benchmark/buffers/buffer-read.js b/benchmark/buffers/buffer-read.js index 339da75befce4d..22f8dcbd3dea12 100644 --- a/benchmark/buffers/buffer-read.js +++ b/benchmark/buffers/buffer-read.js @@ -15,7 +15,9 @@ const types = [ 'FloatLE', 'FloatBE', 'DoubleLE', - 'DoubleBE' + 'DoubleBE', + 'IntLE', + 'IntBE', ]; const bench = common.createBenchmark(main, { @@ -34,11 +36,19 @@ function main(conf) { const fn = `read${type}`; buff.writeDoubleLE(0, 0, noAssert); - const testFunction = new Function('buff', ` - for (var i = 0; i !== ${len}; i++) { - buff.${fn}(0, ${JSON.stringify(noAssert)}); - } - `); + + var call; + if (fn === 'readIntLE' || fn === 'readIntBE') { + call = `buff.${fn}(0, 1, ${JSON.stringify(noAssert)})`; + } else { + call = `buff.${fn}(0, ${JSON.stringify(noAssert)})`; + } + + const testFunction = new Function( + 'buff', + `for (var i = 0; i !== ${len}; ++i) { ${call}; }` + ); + bench.start(); testFunction(buff); bench.end(len / 1e6); diff --git a/doc/api/buffer.md b/doc/api/buffer.md index 07404a043bcc40..5158209ec55aff 100644 --- a/doc/api/buffer.md +++ b/doc/api/buffer.md @@ -1776,8 +1776,11 @@ console.log(buf.readIntLE(0, 6).toString(16)); // Prints: 1234567890ab console.log(buf.readIntBE(0, 6).toString(16)); -// Throws an exception: RangeError: Index out of range +// Throws ERR_INDEX_OUT_OF_RANGE: console.log(buf.readIntBE(1, 6).toString(16)); + +// Throws ERR_OUT_OF_RANGE: +console.log(buf.readIntBE(1, 0).toString(16)); ``` ### buf.readUInt8(offset[, noAssert]) diff --git a/lib/buffer.js b/lib/buffer.js index 180bfe6badaf97..cd08453c243561 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -200,10 +200,11 @@ Buffer.from = function from(value, encodingOrOffset, length) { ); } - if (typeof value === 'number') + if (typeof value === 'number') { throw new errors.TypeError( 'ERR_INVALID_ARG_TYPE', 'value', 'not number', value ); + } const valueOf = value.valueOf && value.valueOf(); if (valueOf !== null && valueOf !== undefined && valueOf !== value) @@ -447,10 +448,11 @@ Buffer[kIsEncodingSymbol] = Buffer.isEncoding; Buffer.concat = function concat(list, length) { var i; - if (!Array.isArray(list)) + if (!Array.isArray(list)) { throw new errors.TypeError( 'ERR_INVALID_ARG_TYPE', 'list', ['Array', 'Buffer', 'Uint8Array'] ); + } if (list.length === 0) return new FastBuffer(); @@ -467,10 +469,11 @@ Buffer.concat = function concat(list, length) { var pos = 0; for (i = 0; i < list.length; i++) { var buf = list[i]; - if (!isUint8Array(buf)) + if (!isUint8Array(buf)) { throw new errors.TypeError( 'ERR_INVALID_ARG_TYPE', 'list', ['Array', 'Buffer', 'Uint8Array'] ); + } _copy(buf, buffer, pos); pos += buf.length; } @@ -1024,6 +1027,14 @@ function checkOffset(offset, ext, length) { throw new errors.RangeError('ERR_INDEX_OUT_OF_RANGE'); } +function checkByteLength(byteLength) { + if (byteLength < 1 || byteLength > 6) { + throw new errors.RangeError('ERR_OUT_OF_RANGE', + 'byteLength', + '>= 1 and <= 6'); + } +} + Buffer.prototype.readUIntLE = function readUIntLE(offset, byteLength, noAssert) { @@ -1109,8 +1120,11 @@ Buffer.prototype.readUInt32BE = function readUInt32BE(offset, noAssert) { Buffer.prototype.readIntLE = function readIntLE(offset, byteLength, noAssert) { offset = offset >>> 0; byteLength = byteLength >>> 0; - if (!noAssert) + + if (!noAssert) { + checkByteLength(byteLength); checkOffset(offset, byteLength, this.length); + } var val = this[offset]; var mul = 1; @@ -1129,8 +1143,11 @@ Buffer.prototype.readIntLE = function readIntLE(offset, byteLength, noAssert) { Buffer.prototype.readIntBE = function readIntBE(offset, byteLength, noAssert) { offset = offset >>> 0; byteLength = byteLength >>> 0; - if (!noAssert) + + if (!noAssert) { + checkByteLength(byteLength); checkOffset(offset, byteLength, this.length); + } var i = byteLength; var mul = 1; @@ -1612,9 +1629,10 @@ if (process.binding('config').hasIntl) { // Transcodes the Buffer from one encoding to another, returning a new // Buffer instance. transcode = function transcode(source, fromEncoding, toEncoding) { - if (!isUint8Array(source)) + if (!isUint8Array(source)) { throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'source', ['Buffer', 'Uint8Array'], source); + } if (source.length === 0) return Buffer.alloc(0); fromEncoding = normalizeEncoding(fromEncoding) || fromEncoding; diff --git a/test/parallel/test-buffer-read.js b/test/parallel/test-buffer-read.js index 88f97db3097b6e..c5b3373cbf23cb 100644 --- a/test/parallel/test-buffer-read.js +++ b/test/parallel/test-buffer-read.js @@ -9,7 +9,7 @@ function read(buff, funx, args, expected) { assert.strictEqual(buff[funx](...args), expected); common.expectsError( - () => buff[funx](-1), + () => buff[funx](-1, args[1]), { code: 'ERR_INDEX_OUT_OF_RANGE' } @@ -142,3 +142,19 @@ assert.throws(() => Buffer.allocUnsafe(8).readFloatLE(-1), RangeError); assert.strictEqual(buf.readIntLE(0, 6), 0x060504030201); assert.strictEqual(buf.readIntBE(0, 6), 0x010203040506); } + +// test for byteLength parameter not between 1 and 6 (inclusive) +common.expectsError(() => { buf.readIntLE(1); }, { code: 'ERR_OUT_OF_RANGE' }); +common.expectsError(() => { buf.readIntLE(1, 'string'); }, + { code: 'ERR_OUT_OF_RANGE' }); +common.expectsError(() => { buf.readIntLE(1, 0); }, + { code: 'ERR_OUT_OF_RANGE' }); +common.expectsError(() => { buf.readIntLE(1, 7); }, + { code: 'ERR_OUT_OF_RANGE' }); +common.expectsError(() => { buf.readIntBE(1); }, { code: 'ERR_OUT_OF_RANGE' }); +common.expectsError(() => { buf.readIntBE(1, 'string'); }, + { code: 'ERR_OUT_OF_RANGE' }); +common.expectsError(() => { buf.readIntBE(1, 0); }, + { code: 'ERR_OUT_OF_RANGE' }); +common.expectsError(() => { buf.readIntBE(1, 7); }, + { code: 'ERR_OUT_OF_RANGE' });