Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto: PBKDF2 works with int not ssize_t #5397

Closed
wants to merge 4 commits into from
Closed

crypto: PBKDF2 works with int not ssize_t #5397

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d924513
crypto: PBKDF2 works with `int` not `ssize_t`
indutny Feb 23, 2016
0edf98a
test: fixes
indutny Feb 25, 2016
ebcb143
..
indutny Mar 1, 2016
25f1bf5
...
indutny Mar 1, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 26 additions & 19 deletions src/node_crypto.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4953,12 +4953,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 @@ -4987,31 +4987,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 @@ -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_;
};


Expand Down Expand Up @@ -5107,10 +5107,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 @@ -5162,8 +5163,14 @@ 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)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should probably write this as:

if (!std::isfinite(raw_keylen) || raw_keylen < 0 || raw_keylen > INT_MAX) {
  // ...
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack.

type_error = "Bad key length";
goto err;
}

keylen = static_cast<int>(raw_keylen);
if (keylen < 0) {
type_error = "Bad key length";
goto err;
}
Expand All @@ -5190,7 +5197,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
View file
Open in desktop
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, '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);
}, /Bad key length/);