Skip to content

Commit

Permalink
crypto: PBKDF2 works with int not ssize_t
Browse files Browse the repository at this point in the history
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
PR-URL: #5397
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Reviewed-By: Ben Noorhduis <info@bnoordhuis.nl>

Conflicts:
	test/parallel/test-crypto-pbkdf2.js
  • Loading branch information
indutny authored and Fishrock123 committed Mar 2, 2016
1 parent 9424fa5 commit 88f3935
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 31 deletions.
43 changes: 24 additions & 19 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "CNNICHashWhitelist.inc"

#include <errno.h>
#include <limits.h> // INT_MAX
#include <math.h>
#include <stdlib.h>
#include <string.h>
Expand Down Expand Up @@ -4940,12 +4941,12 @@ class PBKDF2Request : public AsyncWrap {
PBKDF2Request(Environment* env,
Local<Object> 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),
Expand Down Expand Up @@ -4974,31 +4975,31 @@ class PBKDF2Request : public AsyncWrap {
return digest_;
}

inline ssize_t passlen() const {
inline int passlen() const {
return passlen_;
}

inline char* pass() const {
return pass_;
}

inline ssize_t saltlen() const {
inline int saltlen() const {
return saltlen_;
}

inline char* salt() const {
return salt_;
}

inline ssize_t keylen() const {
inline int keylen() const {
return keylen_;
}

inline char* key() const {
return key_;
}

inline ssize_t iter() const {
inline int iter() const {
return iter_;
}

Expand Down Expand Up @@ -5031,13 +5032,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_;
};


Expand Down Expand Up @@ -5094,10 +5095,11 @@ void PBKDF2(const FunctionCallbackInfo<Value>& 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<Object> obj;

Expand Down Expand Up @@ -5149,12 +5151,15 @@ void PBKDF2(const FunctionCallbackInfo<Value>& 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) ||
raw_keylen > INT_MAX) {
type_error = "Bad key length";
goto err;
}

keylen = static_cast<int>(raw_keylen);

if (args[4]->IsString()) {
node::Utf8Value digest_name(env->isolate(), args[4]);
digest = EVP_get_digestbyname(*digest_name);
Expand All @@ -5177,7 +5182,7 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {
saltlen,
salt,
iter,
static_cast<ssize_t>(keylen));
keylen);

if (args[5]->IsFunction()) {
obj->Set(env->ondone_string(), args[5]);
Expand Down
21 changes: 9 additions & 12 deletions test/parallel/test-crypto-pbkdf2.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,27 +63,24 @@ assert.throws(function() {
// Should not work with Infinity key length
assert.throws(function() {
crypto.pbkdf2('password', 'salt', 1, Infinity, 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, 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, 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, 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, common.fail);
}, /Bad key length/);

0 comments on commit 88f3935

Please sign in to comment.