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

TimothyGu · MylesBorins · commit 59f71f5661ec · 2017-03-28T18:28:53.000-04:00
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>
diff --git a/src/node_buffer.cc b/src/node_buffer.cc
@@ -148,7 +148,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;
@@ -162,7 +163,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);
@@ -793,17 +794,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;
diff --git a/test/parallel/test-buffer-write-noassert.js b/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);
 
   {
     const error = /Int/.test(funx) ?
@@ -37,6 +33,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]));
@@ -57,3 +62,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]));
