[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/