From 3f25cfd18f2bec61b605aea256860e18fefd5b28 Mon Sep 17 00:00:00 2001
From: Johann <git@johann-hofmann.com>
Date: Sun, 4 Oct 2015 11:28:21 +0200
Subject: [PATCH] crypto: check for valid iteration length in pbkdf2

An iteration length of NaN or Infinity is currently silently coerced to
0, which is more or less undefined behaviour. This commit makes NaN and
Infinity invalid iteration parameter values.

We're doing the same checks for other parameters already, so it would
make sense to be consistent here.
---
 src/node_crypto.cc                  |  8 ++++++--
 test/parallel/test-crypto-pbkdf2.js | 27 +++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 873fbfb410db6f..9b74ed3bfa7682 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -5250,6 +5250,7 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {
   int passlen = -1;
   int saltlen = -1;
   double raw_keylen = -1;
+  double raw_iter = -1;
   int keylen = -1;
   int iter = -1;
   PBKDF2Request* req = nullptr;
@@ -5292,12 +5293,15 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {
     goto err;
   }
 
-  iter = args[2]->Int32Value();
-  if (iter < 0) {
+  raw_iter = args[2]->NumberValue();
+  if (raw_iter < 0 || isnan(raw_iter) || isinf(raw_iter) ||
+      raw_iter > INT_MAX) {
     type_error = "Bad iterations";
     goto err;
   }
 
+  iter = static_cast<int>(raw_iter);
+
   if (!args[3]->IsNumber()) {
     type_error = "Key length not a number";
     goto err;
diff --git a/test/parallel/test-crypto-pbkdf2.js b/test/parallel/test-crypto-pbkdf2.js
index c1897c2d69ebdd..c9bc76970aeecf 100644
--- a/test/parallel/test-crypto-pbkdf2.js
+++ b/test/parallel/test-crypto-pbkdf2.js
@@ -84,3 +84,30 @@ assert.throws(function() {
 assert.throws(function() {
   crypto.pbkdf2('password', 'salt', 1, 4073741824, 'sha256', common.fail);
 }, /Bad key length/);
+
+// Should not work with negative iterations
+assert.throws(function() {
+  crypto.pbkdf2('password', 'salt', -1, 1, 'sha256', common.fail);
+}, /Bad iterations/);
+
+// Should not work with Infinity iterations
+assert.throws(function() {
+  crypto.pbkdf2('password', 'salt', Infinity, 1, 'sha256', common.fail);
+}, /Bad iterations/);
+
+// Should not work with -Infinity iterations
+assert.throws(function() {
+  crypto.pbkdf2('password', 'salt', -Infinity, 1, 'sha256', common.fail);
+}, /Bad iterations/);
+
+// Should not work with NaN iterations
+assert.throws(function() {
+  crypto.pbkdf2('password', 'salt', NaN, 1, 'sha256', common.fail);
+}, /Bad iterations/);
+
+// Should not work with an iteration number that does
+// not fit into 32 signed bits
+assert.throws(function() {
+  crypto.pbkdf2('password', 'salt', 4073741824, 1, 'sha256', common.fail);
+}, /Bad iterations/);
+