From d924513118a4223be74bfea2a4aa5c5ef1391eb1 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Tue, 23 Feb 2016 15:53:45 -0500 Subject: [PATCH 1/4] crypto: PBKDF2 works with `int` not `ssize_t` Change types of all PBKDF2 params to `int` as they are `int` in `evp.h`. Check that `raw_keylen` fits into `int` before passing it to OpenSSL. Fix: #5396 --- src/node_crypto.cc | 45 +++++++++++++++++------------ test/parallel/test-crypto-pbkdf2.js | 7 +++++ 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 5fe3632b109fa7..6d8444c6ab869a 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -4953,12 +4953,12 @@ class PBKDF2Request : public AsyncWrap { PBKDF2Request(Environment* env, Local object, const EVP_MD* digest, - ssize_t passlen, + int passlen, char* pass, - ssize_t saltlen, + int saltlen, char* salt, - ssize_t iter, - ssize_t keylen) + int iter, + int keylen) : AsyncWrap(env, object, AsyncWrap::PROVIDER_CRYPTO), digest_(digest), error_(0), @@ -4987,7 +4987,7 @@ class PBKDF2Request : public AsyncWrap { return digest_; } - inline ssize_t passlen() const { + inline int passlen() const { return passlen_; } @@ -4995,7 +4995,7 @@ class PBKDF2Request : public AsyncWrap { return pass_; } - inline ssize_t saltlen() const { + inline int saltlen() const { return saltlen_; } @@ -5003,7 +5003,7 @@ class PBKDF2Request : public AsyncWrap { return salt_; } - inline ssize_t keylen() const { + inline int keylen() const { return keylen_; } @@ -5011,7 +5011,7 @@ class PBKDF2Request : public AsyncWrap { return key_; } - inline ssize_t iter() const { + inline int iter() const { return iter_; } @@ -5044,13 +5044,13 @@ class PBKDF2Request : public AsyncWrap { private: const EVP_MD* digest_; int error_; - ssize_t passlen_; + int passlen_; char* pass_; - ssize_t saltlen_; + int saltlen_; char* salt_; - ssize_t keylen_; + int keylen_; char* key_; - ssize_t iter_; + int iter_; }; @@ -5107,10 +5107,11 @@ void PBKDF2(const FunctionCallbackInfo& args) { const char* type_error = nullptr; char* pass = nullptr; char* salt = nullptr; - ssize_t passlen = -1; - ssize_t saltlen = -1; - double keylen = -1; - ssize_t iter = -1; + int passlen = -1; + int saltlen = -1; + double raw_keylen = -1; + int keylen = -1; + int iter = -1; PBKDF2Request* req = nullptr; Local obj; @@ -5162,8 +5163,14 @@ void PBKDF2(const FunctionCallbackInfo& args) { goto err; } - keylen = args[3]->NumberValue(); - if (keylen < 0 || isnan(keylen) || isinf(keylen)) { + raw_keylen = args[3]->NumberValue(); + if (raw_keylen < 0.0 || isnan(raw_keylen) || isinf(raw_keylen)) { + type_error = "Bad key length"; + goto err; + } + + keylen = static_cast(raw_keylen); + if (keylen < 0) { type_error = "Bad key length"; goto err; } @@ -5190,7 +5197,7 @@ void PBKDF2(const FunctionCallbackInfo& args) { saltlen, salt, iter, - static_cast(keylen)); + 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 cbea3dae4c0d04..1968584cc58c7c 100644 --- a/test/parallel/test-crypto-pbkdf2.js +++ b/test/parallel/test-crypto-pbkdf2.js @@ -87,3 +87,10 @@ assert.throws(function() { }, function(err) { return err instanceof Error && err.message === 'Bad key length'; }); + +// Should not work with key length that does not fit into 32 signed bits +assert.throws(function() { + crypto.pbkdf2('password', 'salt', 1, 4073741824, 'sha256', common.fail); +}, function(err) { + return err instanceof Error && err.message === 'Bad key length'; +}); From 0edf98ad7d24d086048651635d313bc7c7e4c698 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Thu, 25 Feb 2016 18:59:40 -0500 Subject: [PATCH 2/4] test: fixes --- test/parallel/test-crypto-pbkdf2.js | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/test/parallel/test-crypto-pbkdf2.js b/test/parallel/test-crypto-pbkdf2.js index 1968584cc58c7c..a05eb70a8b8bfb 100644 --- a/test/parallel/test-crypto-pbkdf2.js +++ b/test/parallel/test-crypto-pbkdf2.js @@ -63,34 +63,24 @@ assert.throws(function() { // Should not work with Infinity key length assert.throws(function() { crypto.pbkdf2('password', 'salt', 1, Infinity, 'sha256', common.fail); -}, function(err) { - return err instanceof Error && err.message === 'Bad key length'; -}); +}, /Bad key length/); // Should not work with negative Infinity key length assert.throws(function() { crypto.pbkdf2('password', 'salt', 1, -Infinity, 'sha256', common.fail); -}, function(err) { - return err instanceof Error && err.message === 'Bad key length'; -}); +}, /Bad key length/); // Should not work with NaN key length assert.throws(function() { crypto.pbkdf2('password', 'salt', 1, NaN, 'sha256', common.fail); -}, function(err) { - return err instanceof Error && err.message === 'Bad key length'; -}); +}, /Bad key length/); // Should not work with negative key length assert.throws(function() { crypto.pbkdf2('password', 'salt', 1, -1, 'sha256', common.fail); -}, function(err) { - return err instanceof Error && err.message === 'Bad key length'; -}); +}, /Bad key length/); // Should not work with key length that does not fit into 32 signed bits assert.throws(function() { crypto.pbkdf2('password', 'salt', 1, 4073741824, 'sha256', common.fail); -}, function(err) { - return err instanceof Error && err.message === 'Bad key length'; -}); +}, /Bad key length/); From ebcb143740dac8d815f2d6cf39d6ac8240d1a6f1 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Tue, 1 Mar 2016 08:38:39 +0300 Subject: [PATCH 3/4] .. --- src/node_crypto.cc | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 6d8444c6ab869a..0edb959c3d9e81 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -5164,16 +5164,13 @@ void PBKDF2(const FunctionCallbackInfo& args) { } raw_keylen = args[3]->NumberValue(); - if (raw_keylen < 0.0 || isnan(raw_keylen) || isinf(raw_keylen)) { + if (raw_keylen < 0.0 || isnan(raw_keylen) || isinf(raw_keylen) || + raw_keylen > INT_MAX) { type_error = "Bad key length"; goto err; } keylen = static_cast(raw_keylen); - if (keylen < 0) { - type_error = "Bad key length"; - goto err; - } if (args[4]->IsString()) { node::Utf8Value digest_name(env->isolate(), args[4]); From 25f1bf50ef9823771d9c20027f8b2b82f3456249 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Tue, 1 Mar 2016 08:44:58 +0300 Subject: [PATCH 4/4] ... --- src/node_crypto.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 0edb959c3d9e81..68dc7a4351b88b 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -19,6 +19,7 @@ #include "CNNICHashWhitelist.inc" #include +#include // INT_MAX #include #include #include