From a60a104c727f722ac1a42307ca55a1e99d4c7b33 Mon Sep 17 00:00:00 2001 From: ConorDavenport Date: Thu, 30 Jan 2020 12:12:31 +0000 Subject: [PATCH 1/3] 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 --- src/node_buffer.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 9c5e1a8c534d80..3d402335ba3c3d 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -590,10 +590,11 @@ void Fill(const FunctionCallbackInfo& 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 str_obj; size_t str_length; From 52b68821c59b0c07555af8399a5df205ccfd46e5 Mon Sep 17 00:00:00 2001 From: ConorDavenport Date: Fri, 31 Jan 2020 10:38:43 +0000 Subject: [PATCH 2/3] test: remove buffer overflow test Removed test for overflowing buffers after changes in node_buffer.cc Refs: https://github.com/ConorDavenport/node/blob/master/src/node_buffer.cc https://github.com/nodejs/node/issues/31514 --- test/parallel/test-buffer-fill.js | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/test/parallel/test-buffer-fill.js b/test/parallel/test-buffer-fill.js index 4d50bfdd3887fb..bcfe1410bfd4ba 100644 --- a/test/parallel/test-buffer-fill.js +++ b/test/parallel/test-buffer-fill.js @@ -3,7 +3,6 @@ const common = require('../common'); const assert = require('assert'); const { codes: { ERR_OUT_OF_RANGE } } = require('internal/errors'); -const { internalBinding } = require('internal/test/binding'); const SIZE = 28; const buf1 = Buffer.allocUnsafe(SIZE); @@ -325,11 +324,6 @@ 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); - // Make sure "end" is properly checked, even if it's magically mangled using // Symbol.toPrimitive. { @@ -347,11 +341,6 @@ 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); - // Test that bypassing 'length' won't cause an abort. assert.throws(() => { const buf = Buffer.from('w00t'); From 33a26bc7f016939a46f02dc01cf60166f085497f Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 2 Feb 2020 15:42:17 -0800 Subject: [PATCH 3/3] fixup! test: remove buffer overflow test --- test/parallel/test-buffer-fill.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/parallel/test-buffer-fill.js b/test/parallel/test-buffer-fill.js index bcfe1410bfd4ba..6e24b3511e6e1a 100644 --- a/test/parallel/test-buffer-fill.js +++ b/test/parallel/test-buffer-fill.js @@ -3,6 +3,7 @@ const common = require('../common'); const assert = require('assert'); const { codes: { ERR_OUT_OF_RANGE } } = require('internal/errors'); +const { internalBinding } = require('internal/test/binding'); const SIZE = 28; const buf1 = Buffer.allocUnsafe(SIZE); @@ -324,6 +325,13 @@ Buffer.alloc(8, ''); assert.strictEqual(buf.toString(), 'էէէէէ'); } +// 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. { @@ -341,6 +349,13 @@ Buffer.alloc(8, ''); }); } +// 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(() => { const buf = Buffer.from('w00t');