[Bug 6296] New: chpasswd salt has security issues

[bugzilla at busybox.net]

https://bugs.busybox.net/show_bug.cgi?id=6296

           Summary: chpasswd salt has security issues
           Product: Busybox
           Version: unspecified
          Platform: PC
        OS/Version: Linux
            Status: NEW
          Severity: minor
          Priority: P5
         Component: Other
        AssignedTo: unassigned at busybox.net
        ReportedBy: Lothsahn at yahoo.com
                CC: busybox-cvs at busybox.net
   Estimated Hours: 0.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.

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.

-- 
Configure bugmail: https://bugs.busybox.net/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.