From 4c8d96bc30281d77ab556adfe9cc1f91e0db4522 Mon Sep 17 00:00:00 2001 From: Johann Date: Wed, 23 Sep 2015 18:42:20 +0200 Subject: [PATCH] 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: https://github.com/nodejs/node/issues/2987 PR-URL: https://github.com/nodejs/node/pull/3029 Reviewed-By: Ben Noordhuis Reviewed-By: Brian White Reviewed-By: Fedor Indutny Reviewed-By: Sakthipriyan Vairamani --- src/node_crypto.cc | 9 +++++---- test/parallel/test-crypto-pbkdf2.js | 28 ++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index cb5ef574a17a9a..4cfb977f740a0c 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -19,6 +19,7 @@ #include "CNNICHashWhitelist.inc" #include +#include #include #include @@ -4760,7 +4761,7 @@ void PBKDF2(const FunctionCallbackInfo& 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 obj; @@ -4813,8 +4814,8 @@ void PBKDF2(const FunctionCallbackInfo& 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& args) { saltlen, salt, iter, - keylen); + static_cast(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 index 885831bad162e0..51759ca8357ee7 100644 --- 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'; +});