crypto: fix error handling

This fixes multiple cases where the wrong error was returned in case of e.g. a overflow / wrong type. PR-URL: #19445 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
nodejs · Mar 25, 2018 · 333adf6 · 333adf6
1 parent c1278e5
commit 333adf6
Show file tree

Hide file tree

Showing 2 changed files with 111 additions and 269 deletions.
diff --git a/lib/internal/crypto/random.js b/lib/internal/crypto/random.js
@@ -13,37 +13,44 @@ const {
 
 const { kMaxLength } = require('buffer');
 const kMaxUint32 = Math.pow(2, 32) - 1;
+const kMaxPossibleLength = Math.min(kMaxLength, kMaxUint32);
 
-function assertOffset(offset, length) {
-  if (typeof offset !== 'number' || Number.isNaN(offset)) {
-    throw new ERR_INVALID_ARG_TYPE('offset', 'number');
+function assertOffset(offset, elementSize, length) {
+  if (typeof offset !== 'number') {
+    throw new ERR_INVALID_ARG_TYPE('offset', 'number', offset);
   }
 
-  if (offset > kMaxUint32 || offset < 0) {
-    throw new ERR_INVALID_ARG_TYPE('offset', 'uint32');
-  }
+  offset *= elementSize;
 
-  if (offset > kMaxLength || offset > length) {
-    throw new ERR_OUT_OF_RANGE('offset');
+  const maxLength = Math.min(length, kMaxPossibleLength);
+  if (Number.isNaN(offset) || offset > maxLength || offset < 0) {
+    throw new ERR_OUT_OF_RANGE('offset', `>= 0 && <= ${maxLength}`, offset);
   }
+
+  return offset;
 }
 
-function assertSize(size, offset = 0, length = Infinity) {
-  if (typeof size !== 'number' || Number.isNaN(size)) {
-    throw new ERR_INVALID_ARG_TYPE('size', 'number');
+function assertSize(size, elementSize, offset, length) {
+  if (typeof size !== 'number') {
+    throw new ERR_INVALID_ARG_TYPE('size', 'number', size);
   }
 
-  if (size > kMaxUint32 || size < 0) {
-    throw new ERR_INVALID_ARG_TYPE('size', 'uint32');
+  size *= elementSize;
+
+  if (Number.isNaN(size) || size > kMaxPossibleLength || size < 0) {
+    throw new ERR_OUT_OF_RANGE('size',
+                               `>= 0 && <= ${kMaxPossibleLength}`, size);
   }
 
-  if (size + offset > length || size > kMaxLength) {
-    throw new ERR_OUT_OF_RANGE('size');
+  if (size + offset > length) {
+    throw new ERR_OUT_OF_RANGE('size + offset', `<= ${length}`, size + offset);
   }
+
+  return size;
 }
 
 function randomBytes(size, cb) {
-  assertSize(size);
+  assertSize(size, 1, 0, Infinity);
   if (cb !== undefined && typeof cb !== 'function')
     throw new ERR_INVALID_CALLBACK();
   return _randomBytes(size, cb);
@@ -56,17 +63,14 @@ function randomFillSync(buf, offset = 0, size) {
 
   const elementSize = buf.BYTES_PER_ELEMENT || 1;
 
-  offset *= elementSize;
-  assertOffset(offset, buf.byteLength);
+  offset = assertOffset(offset, elementSize, buf.byteLength);
 
   if (size === undefined) {
     size = buf.byteLength - offset;
   } else {
-    size *= elementSize;
+    size = assertSize(size, elementSize, offset, buf.byteLength);
   }
 
-  assertSize(size, offset, buf.byteLength);
-
   return _randomFill(buf, offset, size);
 }
 
@@ -83,20 +87,19 @@ function randomFill(buf, offset, size, cb) {
     size = buf.bytesLength;
   } else if (typeof size === 'function') {
     cb = size;
-    offset *= elementSize;
     size = buf.byteLength - offset;
   } else if (typeof cb !== 'function') {
     throw new ERR_INVALID_CALLBACK();
   }
+
+  offset = assertOffset(offset, elementSize, buf.byteLength);
+
   if (size === undefined) {
     size = buf.byteLength - offset;
   } else {
-    size *= elementSize;
+    size = assertSize(size, elementSize, offset, buf.byteLength);
   }
 
-  assertOffset(offset, buf.byteLength);
-  assertSize(size, offset, buf.byteLength);
-
   return _randomFill(buf, offset, size, cb);
 }