Skip to content

Commit

Permalink
crypto: check for valid iteration length in pbkdf2
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Johann authored and johannhof committed Jun 15, 2016
1 parent 1a1ff77 commit 3f25cfd
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 2 deletions.
8 changes: 6 additions & 2 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
27 changes: 27 additions & 0 deletions test/parallel/test-crypto-pbkdf2.js
Original file line number Diff line number Diff line change
Expand Up @@ -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/);

0 comments on commit 3f25cfd

Please sign in to comment.