KeyForPassword() broken for multi-byte passwords (UTF-8)

[arthurwalasek]

In the code for keyforpassword() you will find:

int result = CCKeyDerivationPBKDF(keySettings.PBKDFAlgorithm,      // algorithm
                   password.UTF8String, // password
                   password.length, // passwordLength
                   ...

The problem is that password.length is the length of the password BEFORE UTF8 encoding, and you are passing the password as a UTF8String. UTF8 encoding will be longer than the original string if using multi-byte characters (use non-english keyboard or hold down a vowel and pick a different symbol).

the password length should be calculated by something like:
const char *utfpass = password.UTF8String;
size_t utfpasslen = strlen(utfpass);

int result = CCKeyDerivationPBKDF(keySettings.PBKDFAlgorithm,      // algorithm
                   utfpass,                // password
                   utfpasslen,                    // passwordLength
                   ...

Unfortunately, keyforpassword() is used during decrypt as well...

[rnapier]

That is a really good catch and a major security issue. The password is being significantly shortened for multibyte passwords.

The fix is very easy:

[password lengthOfBytesUsingEncoding:NSUTF8StringEncoding], // passwordLength

My problem is thinking through how to address it for existing files, since this change would cause files that were encrypted with multibyte characters to no longer decrypt at all.

The obvious answer is to rev the format version so the semantics are clear. I'm just trying to think through if that's the only way to do it correctly.

[arthurwalasek]

I can send you what I did to fix it - it involves when decrypting - if
decrypting fails, to check the 2 password lengths (1 with UTF8 1 without).
If they differ, then decrypt the old way and re-encrypt the new correct
way. I also had to change RNDecryptor for the common header function to
also do it the old way if I set a flag. Basically I made a new
"badkeyforpassword" function that still did it the old way, and called it
instead of the correct one when this condition occurred and I couldn't
decrypt...

On Wed, Aug 21, 2013 at 7:47 PM, Rob Napier notifications@github.comwrote:

That is a really good catch and a major security issue. The password is
being significantly shortened.

The fix is very easy:

[password lengthOfBytesUsingEncoding:NSUTF8StringEncoding], // passwordLength

My problem is thinking through how to address it for existing files, since
this change would cause files that were encrypted with multibyte characters
to no longer decrypt at all.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/77#issuecomment-23065251
.