src, buffer: do not segfault on out-of-range index

Also add test cases for partial writes and invalid indices.

PR-URL: #11927
Fixes: #8724
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

Author: TimothyGu

src/node_buffer.cc

@@ -169,7 +169,8 @@ void CallbackInfo::WeakCallback(Isolate* isolate) {
 // Parse index for external array data.
 inline MUST_USE_RESULT bool ParseArrayIndex(Local<Value> arg,
                                             size_t def,
-                                            size_t* ret) {
+                                            size_t* ret,
+                                            size_t needed = 0) {
   if (arg->IsUndefined()) {
     *ret = def;
     return true;
@@ -183,7 +184,7 @@ inline MUST_USE_RESULT bool ParseArrayIndex(Local<Value> arg,
   // Check that the result fits in a size_t.
   const uint64_t kSizeMax = static_cast<uint64_t>(static_cast<size_t>(-1));
   // coverity[pointless_expression]
-  if (static_cast<uint64_t>(tmp_i) > kSizeMax)
+  if (static_cast<uint64_t>(tmp_i) > kSizeMax - needed)
     return false;
 
   *ret = static_cast<size_t>(tmp_i);
@@ -815,17 +816,28 @@ void WriteFloatGeneric(const FunctionCallbackInfo<Value>& args) {
     CHECK_NE(ts_obj_data, nullptr);
 
   T val = args[1]->NumberValue(env->context()).FromMaybe(0);
-  size_t offset = args[2]->IntegerValue(env->context()).FromMaybe(0);
 
   size_t memcpy_num = sizeof(T);
+  size_t offset;
 
-  if (should_assert) {
-    THROW_AND_RETURN_IF_OOB(offset + memcpy_num >= memcpy_num);
-    THROW_AND_RETURN_IF_OOB(offset + memcpy_num <= ts_obj_length);
+  // If the offset is negative or larger than the size of the ArrayBuffer,
+  // throw an error (if needed) and return directly.
+  if (!ParseArrayIndex(args[2], 0, &offset, memcpy_num) ||
+      offset >= ts_obj_length) {
+    if (should_assert)
+      THROW_AND_RETURN_IF_OOB(false);
+    return;
   }
 
-  if (offset + memcpy_num > ts_obj_length)
-    memcpy_num = ts_obj_length - offset;
+  // If the offset is too large for the entire value, but small enough to fit
+  // part of the value, throw an error and return only if should_assert is
+  // true. Otherwise, write the part of the value that fits.
+  if (offset + memcpy_num > ts_obj_length) {
+    if (should_assert)
+      THROW_AND_RETURN_IF_OOB(false);
+    else
+      memcpy_num = ts_obj_length - offset;
+  }
 
   union NoAlias {
     T val;

test/parallel/test-buffer-write-noassert.js

@@ -4,21 +4,17 @@ const assert = require('assert');
 
 // testing buffer write functions
 
+const outOfRange = /^RangeError: (?:Index )?out of range(?: index)?$/;
+
 function write(funx, args, result, res) {
   {
     const buf = Buffer.alloc(9);
     assert.strictEqual(buf[funx](...args), result);
     assert.deepStrictEqual(buf, res);
   }
 
-  {
-    const invalidArgs = Array.from(args);
-    invalidArgs[1] = -1;
-    assert.throws(
-      () => Buffer.alloc(9)[funx](...invalidArgs),
-      /^RangeError: (?:Index )?out of range(?: index)?$/
-    );
-  }
+  writeInvalidOffset(-1);
+  writeInvalidOffset(9);
 
   if (!/Int/.test(funx)) {
     assert.throws(
@@ -33,6 +29,15 @@ function write(funx, args, result, res) {
     assert.deepStrictEqual(buf2, res);
   }
 
+  function writeInvalidOffset(offset) {
+    const newArgs = Array.from(args);
+    newArgs[1] = offset;
+    assert.throws(() => Buffer.alloc(9)[funx](...newArgs), outOfRange);
+
+    const buf = Buffer.alloc(9);
+    buf[funx](...newArgs, true);
+    assert.deepStrictEqual(buf, Buffer.alloc(9));
+  }
 }
 
 write('writeInt8', [1, 0], 1, Buffer.from([1, 0, 0, 0, 0, 0, 0, 0, 0]));
@@ -53,3 +58,45 @@ write('writeDoubleBE', [1, 1], 9, Buffer.from([0, 63, 240, 0, 0, 0, 0, 0, 0]));
 write('writeDoubleLE', [1, 1], 9, Buffer.from([0, 0, 0, 0, 0, 0, 0, 240, 63]));
 write('writeFloatBE', [1, 1], 5, Buffer.from([0, 63, 128, 0, 0, 0, 0, 0, 0]));
 write('writeFloatLE', [1, 1], 5, Buffer.from([0, 0, 0, 128, 63, 0, 0, 0, 0]));
+
+function writePartial(funx, args, result, res) {
+  assert.throws(() => Buffer.alloc(9)[funx](...args), outOfRange);
+  const buf = Buffer.alloc(9);
+  assert.strictEqual(buf[funx](...args, true), result);
+  assert.deepStrictEqual(buf, res);
+}
+
+// Test partial writes (cases where the buffer isn't large enough to hold the
+// entire value, but is large enough to hold parts of it).
+writePartial('writeIntBE', [0x0eadbeef, 6, 4], 10,
+             Buffer.from([0, 0, 0, 0, 0, 0, 0x0e, 0xad, 0xbe]));
+writePartial('writeIntLE', [0x0eadbeef, 6, 4], 10,
+             Buffer.from([0, 0, 0, 0, 0, 0, 0xef, 0xbe, 0xad]));
+writePartial('writeInt16BE', [0x1234, 8], 10,
+             Buffer.from([0, 0, 0, 0, 0, 0, 0, 0, 0x12]));
+writePartial('writeInt16LE', [0x1234, 8], 10,
+             Buffer.from([0, 0, 0, 0, 0, 0, 0, 0, 0x34]));
+writePartial('writeInt32BE', [0x0eadbeef, 6], 10,
+             Buffer.from([0, 0, 0, 0, 0, 0, 0x0e, 0xad, 0xbe]));
+writePartial('writeInt32LE', [0x0eadbeef, 6], 10,
+             Buffer.from([0, 0, 0, 0, 0, 0, 0xef, 0xbe, 0xad]));
+writePartial('writeUIntBE', [0xdeadbeef, 6, 4], 10,
+             Buffer.from([0, 0, 0, 0, 0, 0, 0xde, 0xad, 0xbe]));
+writePartial('writeUIntLE', [0xdeadbeef, 6, 4], 10,
+             Buffer.from([0, 0, 0, 0, 0, 0, 0xef, 0xbe, 0xad]));
+writePartial('writeUInt16BE', [0x1234, 8], 10,
+             Buffer.from([0, 0, 0, 0, 0, 0, 0, 0, 0x12]));
+writePartial('writeUInt16LE', [0x1234, 8], 10,
+             Buffer.from([0, 0, 0, 0, 0, 0, 0, 0, 0x34]));
+writePartial('writeUInt32BE', [0xdeadbeef, 6], 10,
+             Buffer.from([0, 0, 0, 0, 0, 0, 0xde, 0xad, 0xbe]));
+writePartial('writeUInt32LE', [0xdeadbeef, 6], 10,
+             Buffer.from([0, 0, 0, 0, 0, 0, 0xef, 0xbe, 0xad]));
+writePartial('writeDoubleBE', [1, 2], 10,
+             Buffer.from([0, 0, 63, 240, 0, 0, 0, 0, 0]));
+writePartial('writeDoubleLE', [1, 2], 10,
+             Buffer.from([0, 0, 0, 0, 0, 0, 0, 0, 240]));
+writePartial('writeFloatBE', [1, 6], 10,
+             Buffer.from([0, 0, 0, 0, 0, 0, 63, 128, 0]));
+writePartial('writeFloatLE', [1, 6], 10,
+             Buffer.from([0, 0, 0, 0, 0, 0, 0, 0, 128]));