Skip to content

Commit

Permalink
buffer: check byteLength in readInt(B|L)E
Browse files Browse the repository at this point in the history
The 'byteLength' argument should be required and of type 'number'.
It should have a value between 1 and 6.

PR-URL: nodejs#11146
Fixes: nodejs#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>
  • Loading branch information
seppevs authored and BridgeAR committed Jan 5, 2018
1 parent a9e422e commit d964ffe
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 14 deletions.
22 changes: 16 additions & 6 deletions benchmark/buffers/buffer-read.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ const types = [
'FloatLE',
'FloatBE',
'DoubleLE',
'DoubleBE'
'DoubleBE',
'IntLE',
'IntBE',
];

const bench = common.createBenchmark(main, {
Expand All @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion doc/api/buffer.md
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
30 changes: 24 additions & 6 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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();
Expand All @@ -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;
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
18 changes: 17 additions & 1 deletion test/parallel/test-buffer-read.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
Expand Down Expand Up @@ -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' });

0 comments on commit d964ffe

Please sign in to comment.