[systemd-devel] [PATCH] cryptsetup: check that password is not null

[Thomas H.P. Andersen]

On Thu, Jun 12, 2014 at 11:08 PM, Greg KH <gregkh at linuxfoundation.org> wrote:
> On Thu, Jun 12, 2014 at 10:55:50PM +0200, Thomas H.P. Andersen wrote:
>> From: Thomas Hindoe Paaboel Andersen <phomes at gmail.com>
>>
>> Beef up the assert to protect against passing null to strlen.
>>
>> Found with scan-build.
>> ---
>>  src/cryptsetup/cryptsetup.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/cryptsetup/cryptsetup.c b/src/cryptsetup/cryptsetup.c
>> index 812b32f..a67d85e 100644
>> --- a/src/cryptsetup/cryptsetup.c
>> +++ b/src/cryptsetup/cryptsetup.c
>> @@ -344,7 +344,7 @@ static int attach_tcrypt(struct crypt_device *cd,
>>
>>          assert(cd);
>>          assert(name);
>> -        assert(key_file || passwords);
>> +        assert(key_file || (passwords && passwords[0]));
>
> Shouldn't strlen of an "empty" string just return 0?

Passing null to strlen is undefined behavior and seg faults reliably for me.

> What is this fixing really?

Just a theoretical problem found with static analysis. Not sure if we
can actually hit the problem but the current assert gives a false
sense of security.

> thanks,
>
> greg k-h