[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: util-linux patch bug: too long password



In article <20010107185251.B85325@midten.fast.no>,
	Alexander S A Kjeldaas <Alexander.Kjeldaas@fast.no> writes:
> On Sun, Jan 07, 2001 at 06:22:53PM +0100, Martin Douda wrote:
>> Hi,
>> I'm mailing to you as to mainitainer of kernel international patches. 
>> In current (2.4.0.1) crypto patch is patch for util-linux, containing this
>> lines:
>> 
>> +	case LO_CRYPT_FISH2:
>> +	case LO_CRYPT_BLOW:
>> +	case LO_CRYPT_IDEA:
>> +	case LO_CRYPT_CAST128:
>> +		pass = xgetpass(pfd, _("Password :"));
>> +		strncpy(passwdbuff+1,pass,PASSWDBUFFLEN-1);
>> +		passwdbuff[0] = 'A';
>> +		rmd160_hash_buffer(keybits,pass,strlen(pass));
>> +		rmd160_hash_buffer(keybits+HASHLENGTH,passwdbuff,strlen(pass)+1);
>> 
>> As you probably know, strncpy will not terminate copyied string with \0
>> when copying string of size PASSWDBUFFLEN-1. This may cause strlen(pass)
>> and rmd160_hash_buffer read after end of passwdbuff.
>> 
> 
> Yup, that's a bug.  Thanks for auditing the code!  The bug appears to
> be duplicated in the LO_CRYPT_CRYPTOAPI case statement too.
> 
> astor
> 
In general, just avoid strncpy at all, it being an almost useless
function. It is not guaranteed to 0 terminate, and does (for most
apps) utterly useless zeroing of the whole buffer if the string is
short. Just do a strlen/length check/memcpy/add 0 combo

PS: notice that the last line looks wrong if the copy to passwdbuff
    truncated pass.

I THINK it should be something like (completely untested):

    case LO_CRYPT_FISH2:
    case LO_CRYPT_BLOW:
    case LO_CRYPT_IDEA:
    case LO_CRYPT_CAST128:
            pass = xgetpass(pfd, _("Password :"));
            len  = strlen(pass);
            rmd160_hash_buffer(keybits,pass,len);

            passwdbuff[0] = 'A';
            if (len > PASSWDBUFFLEN-2) len = PASSWDBUFFLEN-2;
            memcpy(passwdbuff+1, pass, len);
            passwdbuff[++len] = 0;
            rmd160_hash_buffer(keybits+HASHLENGTH,passwdbuff,len);

Linux-crypto:  cryptography in and on the Linux system
Archive:       http://mail.nl.linux.org/linux-crypto/