From 949af3dcc0920b6989958f0add77f12fefe6706a Mon Sep 17 00:00:00 2001 From: Matt Loring Date: Mon, 7 Dec 2015 16:52:53 -0700 Subject: [PATCH] buffer: fix range checking for slowToString If `start` is not a valid number in the range, then the default value zero will be used. Same way, if `end` is not a valid number in the accepted range, then, by default, the length of the buffer is assumed. Fixes: https://github.com/nodejs/node/issues/2668 Ref: https://github.com/nodejs/node/pull/2919 PR-URL: https://github.com/nodejs/node/pull/4019 Reviewed-By: Trevor Norris --- lib/buffer.js | 31 ++++++++++++++++++++++++++----- src/node_internals.h | 2 +- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/lib/buffer.js b/lib/buffer.js index 41f1f4b391900b..7221f42c5a8fc9 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -327,13 +327,34 @@ Object.defineProperty(Buffer.prototype, 'offset', { function slowToString(encoding, start, end) { var loweredCase = false; - start = start >>> 0; - end = end === undefined || end === Infinity ? this.length : end >>> 0; + // No need to verify that "this.length <= MAX_UINT32" since it's a read-only + // property of a typed array. + + // This behaves neither like String nor Uint8Array in that we set start/end + // to their upper/lower bounds if the value passed is out of range. + // undefined is handled specially as per ECMA-262 6th Edition, + // Section 13.3.3.7 Runtime Semantics: KeyedBindingInitialization. + if (start === undefined || start < 0) + start = 0; + // Return early if start > this.length. Done here to prevent potential uint32 + // coercion fail below. + if (start > this.length) + return ''; + + if (end === undefined || end > this.length) + end = this.length; + + if (end <= 0) + return ''; + + // Force coersion to uint32. This will also coerce falsey/NaN values to 0. + end >>>= 0; + start >>>= 0; + + if (end <= start) + return ''; if (!encoding) encoding = 'utf8'; - if (start < 0) start = 0; - if (end > this.length) end = this.length; - if (end <= start) return ''; while (true) { switch (encoding) { diff --git a/src/node_internals.h b/src/node_internals.h index bfe4fced26297a..e1ba9e22941e31 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -172,7 +172,7 @@ inline MUST_USE_RESULT bool ParseArrayIndex(v8::Local arg, return true; } - int32_t tmp_i = arg->Int32Value(); + int32_t tmp_i = arg->Uint32Value(); if (tmp_i < 0) return false;