Merge pull request #807 from techhead/known_length

kelektiv · Jun 1, 2020 · 5916a46 · 5916a46
2 parents cf4efd9 + f28e916
commit 5916a46
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 12 deletions.
diff --git 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,11 @@ 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
-		* 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 */
-		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
@@ -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<Napi::String>();
         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
@@ -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 *);
 

diff --git 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");