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.

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 fails silently with NaN bitlength #2987

Closed
coolaj86 opened this issue Sep 21, 2015 · 1 comment
Closed

crypto.pbkdf2 fails silently with NaN bitlength #2987

coolaj86 opened this issue Sep 21, 2015 · 1 comment
Labels
crypto Issues and PRs related to the crypto subsystem.

Comments

@coolaj86
Copy link
Contributor

Here's an example:

    'use strict';

    function sha256(buf) {
      return crypto.createHash('sha256').update(buf).digest('hex');
    }

    var crypto = require('crypto');
    var secret = 'my special secret';
    var appPbkdf2Salt = sha256(new Buffer("MY_SALT"));
    var iterations = 1000;
    var bitLength; // oops, forgot to define bit length
    var keyByteLength = bitLength / 8;
    var hashname = 'sha256';

    crypto.pbkdf2(secret, appPbkdf2Salt, iterations, keyByteLength, hashname, function (err, bytes) {
      if (err) {
        throw err;
      }

      console.log('bytes', bytes.toString('hex'));
    });

I'm assuming that NaN is coerced to 0. I would think that 0 should also throw an error.

@coolaj86 coolaj86 changed the title crypto.pbkdf2 fails silently with NaN length crypto.pbkdf2 fails silently with NaN bitlength Sep 21, 2015
@brendanashworth brendanashworth added the crypto Issues and PRs related to the crypto subsystem. label Sep 21, 2015
johannhof pushed a commit to johannhof/node that referenced this issue Sep 23, 2015
issue nodejs#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: nodejs#2987
@coolaj86
Copy link
Contributor Author

Thank you @johannhof, @brendanashworth!

rvagg pushed a commit that referenced this issue Sep 30, 2015
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: #2987

PR-URL: #3029
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem.
Projects
None yet
Development

No branches or pull requests

2 participants