buffer: check byteLength in readInt(B|L)E

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 <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

4 files changed, 61 insertions, 14 deletions

diff --git a/benchmark/buffers/buffer-read.js b/benchmark/buffers/buffer-read.js
index 339da75bef..22f8dcbd3d 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 07404a043b..5158209ec5 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 180bfe6bad..cd08453c24 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 88f97db309..c5b3373cbf 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' });