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

Completely broken (on Node v8.1.2) #3

Closed
larsch opened this issue Jul 20, 2017 · 2 comments
Closed

Completely broken (on Node v8.1.2) #3

larsch opened this issue Jul 20, 2017 · 2 comments
Assignees

Comments

@larsch
Copy link

larsch commented Jul 20, 2017

This module does not treat strings correctly as binary. No return values are valid. You need to pass 'binary' to Hash.prototype.update, or better yet, use Hash.prototype.digest without an argument (you get a Buffer object instead of a string).

Test:

var cryptmd5 = require('cryptmd5');
var cp = require('child_process');
var salts = [ '0000', 'UwV8', '92AB', '    ' ];
var passwords = [ 'password', '$iB7-tZeYp', '5a;eDIaT6Fq-NQe9:Ypj', 'UHMQUbac:_F%I*?8;HkuJdjsoVtBjzJz.J6?S;$%9,wB.cn.AKV*zrFT&3V6FN8c' ];
salts.forEach(function(salt) {
  passwords.forEach(function(password) {
    var opensslResult = cp.execSync('openssl passwd -1 -salt "' + salt + '" "' + password + '"').toString().trim();
    var cryptmd5Result = cryptmd5.cryptMD5(password, salt);
    console.log(opensslResult, cryptmd5Result, opensslResult == cryptmd5Result ? 'SUCCESS' : 'FAILED');
  });
});

Output:

$1$0000$aWwcZQIpZ/gD70N/fOUeh0 $1$0000$VtjqZ2dMBvsUmNcmmB1Er/ FAILED
$1$0000$xpuwLnUdN2/VgqV1upjQt. $1$0000$ex2.nm24yBAS3XGlKA60b1 FAILED
$1$0000$VVsZ2IjwX4v3spDLkCZsi/ $1$0000$dICRVw3Z6JLiBXSsFCwYM0 FAILED
$1$0000$JihTCFqGpBaZlIasx9cJ5/ $1$0000$3X7fya51Assg59llpHVkc0 FAILED
$1$UwV8$67M.3YMKh8ItI5.yhl2Zf/ $1$UwV8$khQeDiqg4YwEsGxgtduj20 FAILED
$1$UwV8$OhuhXlQvrBdWy4JjXneml. $1$UwV8$uxlS7CzJrekR3y.aRxelI/ FAILED
$1$UwV8$56rzDyLzsvjnH2MokZbrm/ $1$UwV8$6ufPuZ.0TrqpJWy4ttAHG/ FAILED
$1$UwV8$RQa12n7.dU.6upwj6GAkV1 $1$UwV8$u8MVnnJ8d7Yh7v3em54Ol. FAILED
$1$92AB$iMudtmg/UO2dhzZgBudkZ. $1$92AB$krAdB3zeYRH3wAXA51W4q0 FAILED
$1$92AB$QqZ6FsMqz3vz8I5G5fANX/ $1$92AB$IJVwiusUF79LG5.//eEOf1 FAILED
$1$92AB$1/Q9T0GVJfFcYz6U06jZ5. $1$92AB$E7NQo.JwLFfRx1pl2Tabb. FAILED
$1$92AB$kUZpMKw0PwWYMWdH69rtM. $1$92AB$QwgWY4UkWtVo4jgquZUPF0 FAILED
$1$    $XPcscamJTSm22xABupzQ1. $1$    $ijMahtW0Q.QMoyW9owq5F0 FAILED
$1$    $DKKvfQXvMMds4EFnGKQEe. $1$    $5HFfBZIoAuyYnxNwo7bem/ FAILED
$1$    $uvoJq39cchkkbuporqk2l1 $1$    $i7sNIdClAruCm/FZBAC3e/ FAILED
$1$    $/3965JlxdV8oXpaKAb4oY1 $1$    $0BhGzbd.v9e9ve.6EnEv6. FAILED
@BlaM
Copy link
Owner

BlaM commented Jul 21, 2017

Looks as if the default encoding has been changed from utf8 to binary in v6.0.0 https://nodejs.org/api/crypto.html#crypto_hash_update_data_inputencoding

@BlaM BlaM self-assigned this Jul 21, 2017
f4810 added a commit to f4810/cryptMD5-for-javascript that referenced this issue Nov 13, 2017
Patch for issue BlaM#3 . Now it works even with nodejs 6+
BlaM#3
@BlaM
Copy link
Owner

BlaM commented Nov 15, 2017

Thank you for your pull request. Always wanted to do that, but kept forgetting when I was close to my computer.

Updated code is pushed to npm.

@BlaM BlaM closed this as completed Nov 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants