From 6ae20433c9fec27e638751c321ac9a27690218c2 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 30 Jun 2016 11:50:13 +0200 Subject: [PATCH 1/2] src: move ParseArrayIndex() to src/node_buffer.cc It's not used anywhere else so move it out of src/node_internals.h. PR-URL: https://github.com/nodejs/node/pull/7497 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- src/node_buffer.cc | 19 +++++++++++++++++++ src/node_internals.h | 18 ------------------ 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 9ae92d861edb9a..14c83a65c2c1f9 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -193,6 +193,25 @@ void CallbackInfo::WeakCallback(Isolate* isolate) { } +// Parse index for external array data. +inline MUST_USE_RESULT bool ParseArrayIndex(Local arg, + size_t def, + size_t* ret) { + if (arg->IsUndefined()) { + *ret = def; + return true; + } + + int64_t tmp_i = arg->IntegerValue(); + + if (tmp_i < 0) + return false; + + *ret = static_cast(tmp_i); + return true; +} + + // Buffer methods bool HasInstance(Local val) { diff --git a/src/node_internals.h b/src/node_internals.h index ff384231c12e0a..e908da37b1f565 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -175,24 +175,6 @@ inline bool IsBigEndian() { return GetEndianness() == kBigEndian; } -// parse index for external array data -inline MUST_USE_RESULT bool ParseArrayIndex(v8::Local arg, - size_t def, - size_t* ret) { - if (arg->IsUndefined()) { - *ret = def; - return true; - } - - int64_t tmp_i = arg->IntegerValue(); - - if (tmp_i < 0) - return false; - - *ret = static_cast(tmp_i); - return true; -} - class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator { public: inline uint32_t* zero_fill_field() { return &zero_fill_field_; } From 630096bc80f16cfc9602f7cf8d091633928f72c7 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 30 Jun 2016 11:51:17 +0200 Subject: [PATCH 2/2] src: guard against overflow in ParseArrayIndex() ParseArrayIndex() would wrap around large (>=2^32) index values on platforms where sizeof(int64_t) > sizeof(size_t). Ensure that the return value fits in a size_t. PR-URL: https://github.com/nodejs/node/pull/7497 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- src/node_buffer.cc | 5 +++++ test/parallel/test-buffer-alloc.js | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 14c83a65c2c1f9..57bfb87de05b04 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -207,6 +207,11 @@ inline MUST_USE_RESULT bool ParseArrayIndex(Local arg, if (tmp_i < 0) return false; + // Check that the result fits in a size_t. + const uint64_t kSizeMax = static_cast(static_cast(-1)); + if (static_cast(tmp_i) > kSizeMax) + return false; + *ret = static_cast(tmp_i); return true; } diff --git a/test/parallel/test-buffer-alloc.js b/test/parallel/test-buffer-alloc.js index a1f7cae33f8504..42f8e270462c48 100644 --- a/test/parallel/test-buffer-alloc.js +++ b/test/parallel/test-buffer-alloc.js @@ -1462,6 +1462,13 @@ assert.throws(function() { Buffer.from(new ArrayBuffer(0), -1 >>> 0); }, /RangeError: 'offset' is out of bounds/); +// ParseArrayIndex() should reject values that don't fit in a 32 bits size_t. +assert.throws(() => { + const a = Buffer(1).fill(0); + const b = Buffer(1).fill(0); + a.copy(b, 0, 0x100000000, 0x100000001); +}), /out of range index/; + // Unpooled buffer (replaces SlowBuffer) const ubuf = Buffer.allocUnsafeSlow(10); assert(ubuf);