From 1a8185828b70ed5f3bbf83846d4a05e6dc589b83 Mon Sep 17 00:00:00 2001 From: Jonathan Hawkes Date: Sun, 31 May 2020 22:48:09 -0400 Subject: [PATCH 1/3] Pass key_len to bcrypt(). Fix for issues #774, #776 --- src/bcrypt.cc | 8 +++----- src/bcrypt_node.cc | 8 ++++---- src/node_blf.h | 2 +- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/bcrypt.cc b/src/bcrypt.cc index 898ef3ca..ec4b7d9f 100644 --- a/src/bcrypt.cc +++ b/src/bcrypt.cc @@ -146,12 +146,11 @@ bcrypt_gensalt(char minor, u_int8_t log_rounds, u_int8_t *seed, char *gsalt) i.e. $2$04$iwouldntknowwhattosayetKdJ6iFtacBqJdKe6aW7ou */ void -bcrypt(const char *key, const char *salt, char *encrypted) +bcrypt(const char *key, size_t key_len, const char *salt, char *encrypted) { blf_ctx state; u_int32_t rounds, i, k; u_int16_t j; - size_t key_len; u_int8_t salt_len, logr, minor; u_int8_t ciphertext[4 * BCRYPT_BLOCKS+1] = "OrpheanBeholderScryDoubt"; u_int8_t csalt[BCRYPT_MAXSALT]; @@ -215,14 +214,13 @@ bcrypt(const char *key, const char *salt, char *encrypted) decode_base64(csalt, BCRYPT_MAXSALT, (u_int8_t *) salt); salt_len = BCRYPT_MAXSALT; if (minor <= 'a') - key_len = (u_int8_t)(strlen(key) + (minor >= 'a' ? 1 : 0)); + key_len = (u_int8_t)(key_len + (minor >= 'a' ? 1 : 0)); else { - /* strlen() returns a size_t, but the function calls + /* size_t, but the function calls * below result in implicit casts to a narrower integer * type, so cap key_len at the actual maximum supported * length here to avoid integer wraparound */ - key_len = strlen(key); if (key_len > 72) key_len = 72; key_len++; /* include the NUL */ diff --git a/src/bcrypt_node.cc b/src/bcrypt_node.cc index 37c67a6f..a5b6c4fd 100644 --- a/src/bcrypt_node.cc +++ b/src/bcrypt_node.cc @@ -148,7 +148,7 @@ namespace { SetError("Invalid salt. Salt must be in the form of: $Vers$log2(NumRounds)$saltvalue"); } char bcrypted[_PASSWORD_LEN]; - bcrypt(input.c_str(), salt.c_str(), bcrypted); + bcrypt(input.c_str(), input.length(), salt.c_str(), bcrypted); output = std::string(bcrypted); } @@ -185,7 +185,7 @@ namespace { throw Napi::Error::New(env, "Invalid salt. Salt must be in the form of: $Vers$log2(NumRounds)$saltvalue"); } char bcrypted[_PASSWORD_LEN]; - bcrypt(data.c_str(), salt.c_str(), bcrypted); + bcrypt(data.c_str(), data.length(), salt.c_str(), bcrypted); return Napi::String::New(env, bcrypted, strlen(bcrypted)); } @@ -206,7 +206,7 @@ namespace { void Execute() { char bcrypted[_PASSWORD_LEN]; if (ValidateSalt(encrypted.c_str())) { - bcrypt(input.c_str(), encrypted.c_str(), bcrypted); + bcrypt(input.c_str(), input.length(), encrypted.c_str(), bcrypted); result = CompareStrings(bcrypted, encrypted.c_str()); } } @@ -243,7 +243,7 @@ namespace { std::string hash = info[1].As(); char bcrypted[_PASSWORD_LEN]; if (ValidateSalt(hash.c_str())) { - bcrypt(pw.c_str(), hash.c_str(), bcrypted); + bcrypt(pw.c_str(), pw.length(), hash.c_str(), bcrypted); return Napi::Boolean::New(env, CompareStrings(bcrypted, hash.c_str())); } else { return Napi::Boolean::New(env, false); diff --git a/src/node_blf.h b/src/node_blf.h index 770b51a0..2d50a39b 100644 --- a/src/node_blf.h +++ b/src/node_blf.h @@ -125,7 +125,7 @@ u_int32_t Blowfish_stream2word(const u_int8_t *, u_int16_t , u_int16_t *); /* bcrypt functions*/ void bcrypt_gensalt(char, u_int8_t, u_int8_t*, char *); -void bcrypt(const char *, const char *, char *); +void bcrypt(const char *, size_t key_len, const char *, char *); void encode_salt(char *, u_int8_t *, char, u_int16_t, u_int8_t); u_int32_t bcrypt_get_rounds(const char *); From ca1e43b6fcba5376276293fedbf1731806936d70 Mon Sep 17 00:00:00 2001 From: Jonathan Hawkes Date: Sun, 31 May 2020 23:34:48 -0400 Subject: [PATCH 2/3] Add test for embedded NULs --- test/implementation.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/implementation.js b/test/implementation.js index 980dbca0..826e7f9a 100644 --- a/test/implementation.js +++ b/test/implementation.js @@ -26,6 +26,11 @@ module.exports = { assert.strictEqual(bcrypt.hashSync("0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345", "$2b$05$CCCCCCCCCCCCCCCCCCCCC."), "$2b$05$CCCCCCCCCCCCCCCCCCCCC.XxrQqgBi/5Sxuq9soXzDtjIZ7w5pMfK"); assert.done(); }, + test_embedded_nulls: function(assert) { + assert.strictEqual(bcrypt.hashSync("Passw\0rd123", "$2b$05$CCCCCCCCCCCCCCCCCCCCC."), "$2b$05$CCCCCCCCCCCCCCCCCCCCC.VHy/kzL4sCcX3Ib3wN5rNGiRt.TpfxS"); + assert.strictEqual(bcrypt.hashSync("Passw\0 you can literally write anything after the NUL character", "$2b$05$CCCCCCCCCCCCCCCCCCCCC."), "$2b$05$CCCCCCCCCCCCCCCCCCCCC.4vJLJQ6nZ/70INTjjSZWQ0iyUek92tu"); + assert.done(); + }, test_shorten_salt_to_128_bits: function(assert) { assert.strictEqual(bcrypt.hashSync("test", "$2a$10$1234567899123456789012"), "$2a$10$123456789912345678901u.OtL1A1eGK5wmvBKUDYKvuVKI7h2XBu"); assert.strictEqual(bcrypt.hashSync("U*U*", "$2a$05$CCCCCCCCCCCCCCCCCCCCCh"), "$2a$05$CCCCCCCCCCCCCCCCCCCCCeUQ7VjYZ2hd4bLYZdhuPpZMUpEUJDw1S"); From f28e916fc4de51bf7afcd9d5e48c9c6ff2659eac Mon Sep 17 00:00:00 2001 From: Amitosh Swain Mahapatra Date: Mon, 1 Jun 2020 23:41:26 +0530 Subject: [PATCH 3/3] Reword comment --- src/bcrypt.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/bcrypt.cc b/src/bcrypt.cc index ec4b7d9f..bd8c5735 100644 --- a/src/bcrypt.cc +++ b/src/bcrypt.cc @@ -217,9 +217,7 @@ bcrypt(const char *key, size_t key_len, const char *salt, char *encrypted) key_len = (u_int8_t)(key_len + (minor >= 'a' ? 1 : 0)); else { - /* size_t, but the function calls - * below result in implicit casts to a narrower integer - * type, so cap key_len at the actual maximum supported + /* cap key_len at the actual maximum supported * length here to avoid integer wraparound */ if (key_len > 72) key_len = 72;