buffer: validate arg types

lib/buffer.js

@@ -61,7 +61,7 @@ const {
   ERR_NO_LONGER_SUPPORTED,
   ERR_UNKNOWN_ENCODING
 } = require('internal/errors').codes;
-const { validateString } = require('internal/validators');
+const { validateString, validateUint32 } = require('internal/validators');
 
 const internalBuffer = require('internal/buffer');
 
@@ -624,10 +624,16 @@ function stringSlice(buf, encoding, start, end) {
   throw new ERR_UNKNOWN_ENCODING(encoding);
 }
 
-Buffer.prototype.copy =
-  function copy(target, targetStart, sourceStart, sourceEnd) {
-    return _copy(this, target, targetStart, sourceStart, sourceEnd);
-  };
+Buffer.prototype.copy = function(target, targetStart, sourceStart, sourceEnd) {
+  if (!isUint8Array(target)) {
+    throw new ERR_INVALID_ARG_TYPE(
+      'target', ['Buffer', 'Uint8Array'], target);
+  }
+  targetStart = validateUint32(targetStart, 'targetStart');
+  sourceStart = validateUint32(sourceStart, 'sourceStart');
+  sourceEnd = validateUint32(sourceEnd, 'sourceEnd') || this.length;
+  return _copy(this, target, targetStart, sourceStart, sourceEnd);
+};
 
 // No need to verify that "buf.length <= MAX_UINT32" since it's a read-only
 // property of a typed array.
@@ -694,33 +700,10 @@ Buffer.prototype.compare = function compare(target,
   if (arguments.length === 1)
     return _compare(this, target);
 
-  if (targetStart === undefined)
-    targetStart = 0;
-  else if (targetStart < 0)
-    throw new ERR_OUT_OF_RANGE('targetStart', '>= 0', targetStart);
-  else
-    targetStart >>>= 0;
-
-  if (targetEnd === undefined)
-    targetEnd = target.length;
-  else if (targetEnd > target.length)
-    throw new ERR_OUT_OF_RANGE('targetEnd', `<= ${target.length}`, targetEnd);
-  else
-    targetEnd >>>= 0;
-
-  if (sourceStart === undefined)
-    sourceStart = 0;
-  else if (sourceStart < 0)
-    throw new ERR_OUT_OF_RANGE('sourceStart', '>= 0', sourceStart);
-  else
-    sourceStart >>>= 0;
-
-  if (sourceEnd === undefined)
-    sourceEnd = this.length;
-  else if (sourceEnd > this.length)
-    throw new ERR_OUT_OF_RANGE('sourceEnd', `<= ${this.length}`, sourceEnd);
-  else
-    sourceEnd >>>= 0;
+  targetStart = validateUint32(targetStart, 'targetStart');
+  targetEnd = validateUint32(targetEnd, 'targetEnd') || target.length;
+  sourceStart = validateUint32(sourceStart, 'sourceStart');
+  sourceEnd = validateUint32(sourceEnd, 'sourceEnd') || this.length;
 
   if (sourceStart >= sourceEnd)
     return (targetStart >= targetEnd ? 0 : -1);

test/parallel/test-buffer-alloc.js

@@ -964,7 +964,8 @@ common.expectsError(
   {
     code: 'ERR_INVALID_ARG_TYPE',
     type: TypeError,
-    message: 'argument must be a buffer'
+    message: 'The "target" argument must be one of type Buffer or ' +
+    'Uint8Array. Received type undefined'
   });
 
 assert.throws(() => Buffer.from(), {

test/parallel/test-buffer-compare.js

@@ -45,3 +45,128 @@ common.expectsError(() => Buffer.alloc(1).compare('abc'), {
   message: 'The "target" argument must be one of ' +
            'type Buffer or Uint8Array. Received type string'
 });
+
+
+// Test compare with offset
+{
+  const a = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 0]);
+  const b = Buffer.from([5, 6, 7, 8, 9, 0, 1, 2, 3, 4]);
+
+  assert.strictEqual(a.compare(b), -1);
+
+// Equivalent to a.compare(b).
+  assert.strictEqual(a.compare(b, 0), -1);
+  assert.strictEqual(a.compare(b, '0'), -1);
+  assert.strictEqual(a.compare(b, undefined), -1);
+
+// Equivalent to a.compare(b).
+  assert.strictEqual(a.compare(b, 0, undefined, 0), -1);
+
+// Zero-length target, return 1
+  assert.strictEqual(a.compare(b, 0, 0, 0), 1);
+  assert.strictEqual(a.compare(b, '0', '0', '0'), 1);
+
+// Equivalent to Buffer.compare(a, b.slice(6, 10))
+  assert.strictEqual(a.compare(b, 6, 10), 1);
+
+// Zero-length source, return -1
+  assert.strictEqual(a.compare(b, 6, 10, 0, 0), -1);
+
+// Zero-length source and target, return 0
+  assert.strictEqual(a.compare(b, 0, 0, 0, 0), 0);
+  assert.strictEqual(a.compare(b, 1, 1, 2, 2), 0);
+
+// Equivalent to Buffer.compare(a.slice(4), b.slice(0, 5))
+  assert.strictEqual(a.compare(b, 0, 5, 4), 1);
+
+// Equivalent to Buffer.compare(a.slice(1), b.slice(5))
+  assert.strictEqual(a.compare(b, 5, undefined, 1), 1);
+
+// Equivalent to Buffer.compare(a.slice(2), b.slice(2, 4))
+  assert.strictEqual(a.compare(b, 2, 4, 2), -1);
+
+// Equivalent to Buffer.compare(a.slice(4), b.slice(0, 7))
+  assert.strictEqual(a.compare(b, 0, 7, 4), -1);
+
+// Equivalent to Buffer.compare(a.slice(4, 6), b.slice(0, 7));
+  assert.strictEqual(a.compare(b, 0, 7, 4, 6), -1);
+
+// zero length target
+  assert.strictEqual(a.compare(b, 0, null), 1);
+
+// coerces to targetEnd == 5
+  assert.strictEqual(a.compare(b, 0, { valueOf: () => 5 }), -1);
+
+// zero length target
+  assert.strictEqual(a.compare(b, Infinity, -Infinity), 1);
+
+// zero length target because default for targetEnd <= targetSource
+  assert.strictEqual(a.compare(b, '0xff'), 1);
+
+  const oor = common.expectsError({ code: 'ERR_OUT_OF_RANGE' }, 7);
+
+  assert.throws(() => a.compare(b, 0, 100, 0), oor);
+  assert.throws(() => a.compare(b, 0, 1, 0, 100), oor);
+  assert.throws(() => a.compare(b, -1), oor);
+  assert.throws(() => a.compare(b, 0, '0xff'), oor);
+  assert.throws(() => a.compare(b, 0, Infinity), oor);
+  assert.throws(() => a.compare(b, 0, 1, -1), oor);
+  assert.throws(() => a.compare(b, -Infinity, Infinity), oor);
+  common.expectsError(() => a.compare(), {
+    code: 'ERR_INVALID_ARG_TYPE',
+    type: TypeError,
+    message: 'The "target" argument must be one of ' +
+      'type Buffer or Uint8Array. Received type undefined'
+  });
+
+  // Validate arg type checking.
+  // Refs: https://github.com/nodejs/node/issues/23668
+  {
+    const expected = {
+      code: 'ERR_INVALID_ARG_TYPE',
+      type: TypeError,
+    }
+
+    common.expectsError(
+      () => b.compare(c, '1'),
+      expected
+    );
+    common.expectsError(
+      () => b.compare(c, 0, '1'),
+      expected
+    );
+    common.expectsError(
+      () => b.compare(c, 0, 0, '1'),
+      expected
+    );
+    common.expectsError(
+      () => b.compare(c, 0, 0, 0, '1'),
+      expected
+    );
+  }
+
+  // Extra! Validate arg range checking.
+  // Refs: https://github.com/nodejs/node/issues/23668
+  {
+    const expected = {
+      code: 'ERR_OUT_OF_RANGE',
+      type: RangeError,
+    }
+    common.expectsError(
+      () => b.copy(c, -1),
+      expected
+    );
+    common.expectsError(
+      () => b.copy(c, 0, -1),
+      expected
+    );
+    common.expectsError(
+      () => b.copy(c, 0, 0, -1),
+      expected
+    );
+    common.expectsError(
+      () => b.copy(c, 0, 0, 0, -1),
+      expected
+    );
+  }
+}

test/parallel/test-buffer-copy.js

@@ -1,6 +1,6 @@
 'use strict';
 
-require('../common');
+const common = require('../common');
 const assert = require('assert');
 
 const b = Buffer.allocUnsafe(1024);
@@ -144,3 +144,48 @@ assert.strictEqual(b.copy(c, 512, 0, 10), 0);
     assert.strictEqual(c[i], e[i]);
   }
 }
+
+
+// Validate arg type checking.
+// Refs: https://github.com/nodejs/node/issues/23668
+{
+  const expected = {
+    code: 'ERR_INVALID_ARG_TYPE',
+    type: TypeError,
+  }
+
+  common.expectsError(
+    () => b.copy(c, '1'),
+    expected
+  );
+  common.expectsError(
+    () => b.copy(c, 0, '1'),
+    expected
+  );
+  common.expectsError(
+    () => b.copy(c, 0, 0, '1'),
+  );
+}
+
+
+// Validate arg range checking.
+// Refs: https://github.com/nodejs/node/issues/23668
+{
+  const expected = {
+    code: 'ERR_OUT_OF_RANGE',
+    type: RangeError,
+  }
+
+  common.expectsError(
+    () => b.copy(c, -1),
+    expected
+  );
+  common.expectsError(
+    () => b.copy(c, 0, -1),
+    expected
+  );
+  common.expectsError(
+    () => b.copy(c, 0, 0, -1),
+    expected
+  );
+}