[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Sun, 23 Dec 2012 18:05:09 +0100
From: magnum <john.magnum@...hmail.com>
To: john-dev@...ts.openwall.com
Subject: Re: scan-build results, part 1
On 23 Dec, 2012, at 17:54 , Dhiru Kholia <dhiru.kholia@...il.com> wrote:
> On Sun, Dec 23, 2012 at 10:14 PM, magnum <john.magnum@...hmail.com> wrote:
>> On 23 Dec, 2012, at 17:34 , Dhiru Kholia <dhiru.kholia@...il.com> wrote:
>>
>>> On Sun, Dec 23, 2012 at 9:46 PM, magnum <john.magnum@...hmail.com> wrote:
>>>> Another weird complaint is for MSCHAPv2. The pos pointer is set to non-null in line 429. How could it ever be a null dereference in line 433?
>>>
>>> I have received an "official" answer on this one. It is *not* a false
>>> positive. "ciphertext" can be NULL and we haven't checked for it
>>> before doing pointer arithmetic on line 429.
>>> gwynne> From the analyzer's point of view, NULL acts like NaN; i.e.
>>> "NULL + anything = NULL" in terms of pointer validity.
>>>
>>> Following patch makes this problem go away.
>>>
>>> diff --git a/src/MSCHAPv2_fmt_plug.c b/src/MSCHAPv2_fmt_plug.c
>>> index d946036..b4c7fcf 100644
>>> --- a/src/MSCHAPv2_fmt_plug.c
>>> +++ b/src/MSCHAPv2_fmt_plug.c
>>> @@ -44,6 +44,7 @@
>>>
>>> #include "sha.h"
>>> #include <openssl/des.h>
>>> +#include <assert.h>
>>> ...
>>
>> Fair enough. But I think I recall Solar wants to avoid using assert() and in this case, we know (even if scan-build doesn't) that this function will never be called with ciphertext being null. So maybe we should just ignore it.
>
> Sure, no problem.
Just to mute scan-build, we could add this instead of that assertion though:
if (!ciphertext)
return ciphertext;
magnum
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
