diff --git a/src/node_buffer.cc b/src/node_buffer.cc index a90e7a5ede6dfe..d2bf5ab1046f56 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -580,7 +580,8 @@ void Fill(const FunctionCallbackInfo& args) { Local str_obj; size_t str_length; enum encoding enc; - CHECK(fill_length + start <= ts_obj_length); + THROW_AND_RETURN_IF_OOB(start <= end); + THROW_AND_RETURN_IF_OOB(fill_length + start <= ts_obj_length); // First check if Buffer has been passed. if (Buffer::HasInstance(args[1])) { diff --git a/test/parallel/test-buffer-fill.js b/test/parallel/test-buffer-fill.js index e5581b2d83d041..c61ad59d7e9cfe 100644 --- a/test/parallel/test-buffer-fill.js +++ b/test/parallel/test-buffer-fill.js @@ -314,3 +314,79 @@ Buffer.alloc(8, ''); buf.fill('է'); assert.strictEqual(buf.toString(), 'էէէէէ'); } + +// Testing public API. Make sure "start" is properly checked, even if it's +// magically mangled using Symbol.toPrimitive. +{ + let elseWasLast = false; + assert.throws(() => { + var ctr = 0; + const start = { + [Symbol.toPrimitive]() { + // We use this condition to get around the check in lib/buffer.js + if (ctr <= 0) { + elseWasLast = false; + ctr = ctr + 1; + return 0; + } else { + elseWasLast = true; + // Once buffer.js calls the C++ implemenation of fill, return -1 + return -1; + } + } + }; + Buffer.alloc(1).fill(Buffer.alloc(1), start, 1); + }, /out of range index/); + // Make sure -1 is making it to Buffer::Fill(). + assert.ok(elseWasLast, + 'internal API changed, -1 no longer in correct location'); +} + +// Testing process.binding. Make sure "start" is properly checked for -1 wrap +// around. +assert.throws(() => { + process.binding('buffer').fill(Buffer.alloc(1), 1, -1, 0, 1); +}, /out of range index/); + +// Make sure "end" is properly checked, even if it's magically mangled using +// Symbol.toPrimitive. +{ + let elseWasLast = false; + assert.throws(() => { + var ctr = 0; + const end = { + [Symbol.toPrimitive]() { + // We use this condition to get around the check in lib/buffer.js + if (ctr <= 1) { + elseWasLast = false; + ctr = ctr + 1; + return 1; + } else { + elseWasLast = true; + // Once buffer.js calls the C++ implemenation of fill, return -1 + return -1; + } + } + }; + Buffer.alloc(1).fill(Buffer.alloc(1), 0, end); + }); + // Make sure -1 is making it to Buffer::Fill(). + assert.ok(elseWasLast, + 'internal API changed, -1 no longer in correct location'); +} + +// Testing process.binding. Make sure "end" is properly checked for -1 wrap +// around. +assert.throws(() => { + process.binding('buffer').fill(Buffer.alloc(1), 1, 1, -2, 1); +}, /out of range index/); + +// Test that bypassing 'length' won't cause an abort. +assert.throws(() => { + const buf = new Buffer('w00t'); + Object.defineProperty(buf, 'length', { + value: 1337, + enumerable: true + }); + buf.fill(''); +});