The chpasswd command gets the salt value from the stack. That is, it's declared in chpasswd_main as: char salt[sizeof("$N$XXXXXXXX")]; After this instruction, salt is never initialized (unless using md5sum mode)--at which point the first 3 characters are set to $1$. The salt is then passed to pw_encrypt, which uses it. It seems like, on a lot of linux systems, we'd be far better off using a random salt from /dev/random or /dev/urandom rather than just directly off the stack. It's likely possible to infer what the value of the salt is off the stack. The second problem this introduces is that when we call pw_encrypt, it immediately calls my_crypt(clear, salt). The behavior of my_crypt is based on the salt. If the salt starts with "$1$" , then it assumes md5 encryption, otherwise it uses either SHA or DES based on what's available on the system. However, this means that there is a VERY small chance (1/256^3) that a user could end up with md5 crypt instead of SHA/DES simply because the "random" stack data started with $1$. The affected code is: if (salt[0] == '$' && salt[1] && salt[2] == '$') { if (salt[1] == '1') return md5_crypt(xzalloc(MD5_OUT_BUFSIZE), (unsigned char*)key, (unsigned char*)salt); It seems like the encryption algorithm (md5) should be passed to these functions as a boolean, rather than as a component of the salt's contents itself. Furthermore, the salt should contain random data.
(In reply to comment #0) > The chpasswd command gets the salt value from the stack. That is, it's > declared in chpasswd_main as: > char salt[sizeof("$N$XXXXXXXX")]; > > After this instruction, salt is never initialized (unless using md5sum > mode)--at which point the first 3 characters are set to $1$. The salt is then > passed to pw_encrypt, which uses it. > It seems like, on a lot of linux systems, we'd be far better off using a random > salt from /dev/random or /dev/urandom rather than just directly off the stack. > It's likely possible to infer what the value of the salt is off the stack. It is initialized by crypt_make_salt(): if (!(opt & OPT_ENC)) { char salt[sizeof("$N$XXXXXXXX")]; crypt_make_salt(salt, 1); <==== if (opt & OPT_MD5) { salt[0] = '$'; salt[1] = '1'; salt[2] = '$'; crypt_make_salt(salt + 3, 4); <==== } free_me = pass = pw_encrypt(pass, salt, 0); } which sets two first bytes to random chars from the "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz" string. If OPT_MD5, then salt is set to "$1$rrrrrrrr" where r's are similarly generated 8 random chars. (The string isn't NUL terminated, it's not a bug). Am I missing something?