buffer: prevent string write offset overflow

Reject offsets outside the destination buffer before subtracting from
its length in SlowWriteString. Normalize wrapper arguments once so
validated values reach the native binding.

Signed-off-by: Matteo Collina <hello@matteocollina.com>

Author: mcollina

lib/internal/buffer.js

@@ -959,30 +959,39 @@ function writeFloatBackwards(val, offset = 0) {
 
 class FastBuffer extends Uint8Array {}
 
-function asciiWrite(buf, string, offset = 0, length = buf.byteLength - offset) {
+function asciiWrite(buf, string, offset = 0, length) {
+  offset = Number(offset);
   if (offset < 0 || offset > buf.byteLength) {
     throw new ERR_BUFFER_OUT_OF_BOUNDS('offset');
   }
+  if (length === undefined) length = buf.byteLength - offset;
+  else length = Number(length);
   if (length < 0 || length > buf.byteLength - offset) {
     throw new ERR_BUFFER_OUT_OF_BOUNDS('length');
   }
   return asciiWriteStatic(buf, string, offset, length);
 }
 
-function latin1Write(buf, string, offset = 0, length = buf.byteLength - offset) {
+function latin1Write(buf, string, offset = 0, length) {
+  offset = Number(offset);
   if (offset < 0 || offset > buf.byteLength) {
     throw new ERR_BUFFER_OUT_OF_BOUNDS('offset');
   }
+  if (length === undefined) length = buf.byteLength - offset;
+  else length = Number(length);
   if (length < 0 || length > buf.byteLength - offset) {
     throw new ERR_BUFFER_OUT_OF_BOUNDS('length');
   }
   return latin1WriteStatic(buf, string, offset, length);
 }
 
-function utf8Write(buf, string, offset = 0, length = buf.byteLength - offset) {
+function utf8Write(buf, string, offset = 0, length) {
+  offset = Number(offset);
   if (offset < 0 || offset > buf.byteLength) {
     throw new ERR_BUFFER_OUT_OF_BOUNDS('offset');
   }
+  if (length === undefined) length = buf.byteLength - offset;
+  else length = Number(length);
   if (length < 0 || length > buf.byteLength - offset) {
     throw new ERR_BUFFER_OUT_OF_BOUNDS('length');
   }

src/node_buffer.cc

@@ -1727,6 +1727,11 @@ void SlowWriteString(const FunctionCallbackInfo<Value>& args) {
   size_t max_length = 0;
 
   THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[2], 0, &offset));
+  if (offset > ts_obj_length) {
+    return node::THROW_ERR_BUFFER_OUT_OF_BOUNDS(
+        env, "\"offset\" is outside of buffer bounds");
+  }
+
   THROW_AND_RETURN_IF_OOB(
       ParseArrayIndex(env, args[3], ts_obj_length - offset, &max_length));
 

test/parallel/test-buffer-write.js

@@ -127,3 +127,17 @@ assert.throws(() => {
 }, common.expectsError({
   code: 'ERR_BUFFER_OUT_OF_BOUNDS',
 }));
+
+for (const method of ['asciiWrite', 'latin1Write', 'utf8Write']) {
+  let calls = 0;
+  const offset = {
+    valueOf() {
+      calls++;
+      return 2;
+    },
+  };
+  assert.throws(() => Buffer.alloc(1)[method]('ww', offset, 1), common.expectsError({
+    code: 'ERR_BUFFER_OUT_OF_BOUNDS',
+  }));
+  assert.strictEqual(calls, 1);
+}