crypto: add more keylen sanity checks in pbkdf2

issue #2987 makes the point that crypto.pbkdf2 should not fail silently and accept invalid but numeric values like NaN and Infinity. We already check if the keylen is lower than 0, so extending that to NaN and Infinity should make sense. Fixes: #2987 PR-URL: #3029 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Brian White <[email protected]> Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
nodejs · Sep 30, 2015 · 4c8d96b · 4c8d96b
1 parent 798dad2
commit 4c8d96b
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 4 deletions.
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -19,6 +19,7 @@
 #include "CNNICHashWhitelist.inc"
 
 #include <errno.h>
+#include <math.h>
 #include <stdlib.h>
 #include <string.h>
 
@@ -4760,7 +4761,7 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {
   char* salt = nullptr;
   ssize_t passlen = -1;
   ssize_t saltlen = -1;
-  ssize_t keylen = -1;
+  double keylen = -1;
   ssize_t iter = -1;
   PBKDF2Request* req = nullptr;
   Local<Object> obj;
@@ -4813,8 +4814,8 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {
     goto err;
   }
 
-  keylen = args[3]->Int32Value();
-  if (keylen < 0) {
+  keylen = args[3]->NumberValue();
+  if (keylen < 0 || isnan(keylen) || isinf(keylen)) {
     type_error = "Bad key length";
     goto err;
   }
@@ -4841,7 +4842,7 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {
                           saltlen,
                           salt,
                           iter,
-                          keylen);
+                          static_cast<ssize_t>(keylen));
 
   if (args[5]->IsFunction()) {
     obj->Set(env->ondone_string(), args[5]);

diff --git a/test/parallel/test-crypto-pbkdf2.js b/test/parallel/test-crypto-pbkdf2.js
@@ -59,3 +59,31 @@ function ondone(err, key) {
 assert.throws(function() {
   crypto.pbkdf2('password', 'salt', 1, 20, null);
 });
+
+// Should not work with Infinity key length
+assert.throws(function() {
+  crypto.pbkdf2('password', 'salt', 1, Infinity, assert.fail);
+}, function(err) {
+  return err instanceof Error && err.message === 'Bad key length';
+});
+
+// Should not work with negative Infinity key length
+assert.throws(function() {
+  crypto.pbkdf2('password', 'salt', 1, -Infinity, assert.fail);
+}, function(err) {
+  return err instanceof Error && err.message === 'Bad key length';
+});
+
+// Should not work with NaN key length
+assert.throws(function() {
+  crypto.pbkdf2('password', 'salt', 1, NaN, assert.fail);
+}, function(err) {
+  return err instanceof Error && err.message === 'Bad key length';
+});
+
+// Should not work with negative key length
+assert.throws(function() {
+  crypto.pbkdf2('password', 'salt', 1, -1, assert.fail);
+}, function(err) {
+  return err instanceof Error && err.message === 'Bad key length';
+});