From 43e2c2e6439ebbf89d92afbfeae16235636dac02 Mon Sep 17 00:00:00 2001
From: ConorDavenport <cnrdavenport@gmail.com>
Date: Thu, 30 Jan 2020 12:12:31 +0000
Subject: [PATCH] src: change Fill() to use ParseArrayIndex()

Changed Fill() to use ParseArrayIndex() when getting start and end
of buffers instead of Uint32Value, supporting buffers of greater
than 2**32

Fixes: https://github.com/nodejs/node/issues/31514
Co-Authored-By: Rich Trott <rtrott@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/31591
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 src/node_buffer.cc                |  9 +++++----
 test/parallel/test-buffer-fill.js | 20 ++++++++++++--------
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/src/node_buffer.cc b/src/node_buffer.cc
index cebeeaade13134..68710f793dd76a 100644
--- a/src/node_buffer.cc
+++ b/src/node_buffer.cc
@@ -568,10 +568,11 @@ void Fill(const FunctionCallbackInfo<Value>& args) {
   THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
   SPREAD_BUFFER_ARG(args[0], ts_obj);
 
-  uint32_t start;
-  if (!args[2]->Uint32Value(ctx).To(&start)) return;
-  uint32_t end;
-  if (!args[3]->Uint32Value(ctx).To(&end)) return;
+  size_t start;
+  THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[2], 0, &start));
+  size_t end;
+  THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[3], 0, &end));
+
   size_t fill_length = end - start;
   Local<String> str_obj;
   size_t str_length;
diff --git a/test/parallel/test-buffer-fill.js b/test/parallel/test-buffer-fill.js
index 4d50bfdd3887fb..6e24b3511e6e1a 100644
--- a/test/parallel/test-buffer-fill.js
+++ b/test/parallel/test-buffer-fill.js
@@ -325,10 +325,12 @@ Buffer.alloc(8, '');
   assert.strictEqual(buf.toString(), 'էէէէէ');
 }
 
-// Testing process.binding. Make sure "start" is properly checked for -1 wrap
-// around.
-assert.strictEqual(
-  internalBinding('buffer').fill(Buffer.alloc(1), 1, -1, 0, 1), -2);
+// Testing process.binding. Make sure "start" is properly checked for range
+// errors.
+assert.throws(
+  () => { internalBinding('buffer').fill(Buffer.alloc(1), 1, -1, 0, 1); },
+  { code: 'ERR_OUT_OF_RANGE' }
+);
 
 // Make sure "end" is properly checked, even if it's magically mangled using
 // Symbol.toPrimitive.
@@ -347,10 +349,12 @@ assert.strictEqual(
   });
 }
 
-// Testing process.binding. Make sure "end" is properly checked for -1 wrap
-// around.
-assert.strictEqual(
-  internalBinding('buffer').fill(Buffer.alloc(1), 1, 1, -2, 1), -2);
+// Testing process.binding. Make sure "end" is properly checked for range
+// errors.
+assert.throws(
+  () => { internalBinding('buffer').fill(Buffer.alloc(1), 1, 1, -2, 1); },
+  { code: 'ERR_OUT_OF_RANGE' }
+);
 
 // Test that bypassing 'length' won't cause an abort.
 assert.throws(() => {