sha256 password encryption output too short

[ischommer]

There are a couple security issues with Silverstripe/Sapphire r53472 in Security::encrypt_password(). The use of base_convert() only uses the first few bytes of a string, truncating the effective length of the salt, and password hash, as can been seen from

<?php

$password = hash('sha256', 'mysecretpassword'); echo $password, "\n"; $password = base_convert($password, 16, 36); echo $password, "\n"; echo base_convert($password, 36, 16), "\n"; //

convert it back to original base ?>

Output: 94aefb8be78b2b7c344d11d1ba8a79ef087eceb19150881f69460b8772753263 3peoppyevskkwoo4g88sgc844ogc8ggcskgocgkwccwg0wc88o 94aefb8be78b1800000000000000000000000000000000000000000000000000

base_convert() is used in two places. The first is in the salt generation, http://open.silverstripe.com/browser/modules/sapphire/trunk/security/ Security.php?rev=53472#L785

and the second is the password hashing http://open.silverstripe.com/browser/modules/sapphire/trunk/security/ Security.php?rev=53472#L799

this makes the hash less secure, any advantage from using longer hash functions is somewhat negated.

Simple fix would be to get the hash function(s) return a binary string as opposed to hexadecimal, and use base64_encode(), and make sure not using case insenstive comparisions.

Migrating from the old password hash, to the corrected one, could be done when a user successfully logins in with the old password hash.

[sminnee]

IMO, the whole use of base_convert() does more harm than good. If people really need a 256 byte hash for their encryption, then they can use a bigger text field to contain it.

The PHP base conversion functions (hexdec and decbin) seem to have precision errors also.