[botan-devel] Bug in SHA-160 with resize to zero not allocated empty buffer

William K. Foster wkf at alum.mit.edu
Mon Jun 23 23:00:56 EDT 2014


Looking at the code, I've determined it is a bug in my code. I was copying
a SHA_160_SSE2 as a base class SHA_160 object, hence the W was size zero
and the compress_n() of SHA_160 got SEGV on NULL W being dereferenced.

That being said, it comes down to a feature I need that I have worked
around for now (which led to my bug) that seems missing in Botan.

The feature I need is to compute interim hashes along the way.  For
example, if I am computing a hash on a 100MB file, I might want to compute
interim hashes as 10MB intervals along the way. It does not appear Botan
supports this, so for now I've been working around this by copying the
HashFunction object and calling final() on the copy at the 10MB intervals,
which allows the original to keep going until the end of the file at 100MB
with furher update() calls since once final() is done, no more calls to
update() are allowed.

Perhaps there is a better way to accomplish this?  Does anyone have any
suggestions here?

In summary, I'd like to efficiently compute a hash value in a single pass
of a file at various intervals throughout the file until the end of the
file.

For now, I've changed my work around to create a new virtual function
called cloneCopy() that does a copy constructor of the object of the
correct class (e.g. SHA_160_SSE2 instead of base class SHA_160).



On Mon, Jun 23, 2014 at 6:18 PM, William K. Foster <wkf at alum.mit.edu> wrote:

> I have found that my proposed fix leads to a memory leak due to:
>
>       void deallocate(T* p, size_t n)
>          { if(alloc && p && n) alloc->deallocate(p, sizeof(T)*n); }
>
> skipping the deallocate of zero length.
>
> This makes me think that the original solution even if fixed here too
> would lead to performance degradation.
>
>  What would be the proper solution to the original SEGV I faced with
> SHA-160?  Other hashes work fine, it seems only SHA-160 gets SEGVs due to
> null pointer dereference.
>
>
>
>
> On Wed, Jun 11, 2014 at 12:32 PM, William K. Foster <wkf at alum.mit.edu>
> wrote:
>
>> No comments on this proposed bug fix?
>>
>>
>> On Tue, Jun 3, 2014 at 3:08 PM, William K. Foster <wkf at alum.mit.edu>
>> wrote:
>>
>>> Hello,
>>>
>>> I have come across a SEGV due to following a null pointer in the SHA-160
>>> implementation for Botan v1.10.8.  The failure is seen in
>>> SHA_160::compress_n() being called when W has a null buf and the null
>>> pointer is dereferenced.  The SHA_160 is constructed by SHA_160_SSE2()
>>> in file src/hash/sha1_sse2/sha1_sse2.h line 22 where the SHA_160(0) is
>>> used, overriding the size to be zero. This becomes an issue during
>>> construction where Botan::MemoryRegion<unsigned int>::resize() in file
>>> src/alloc/secmem.h line 213 the code is reached with a size of n being
>>> zero but allocated is also zero, and thus the conditional takes the
>>> path of keeping the existing allocation for buf (a null pointer) for
>>> the requested zero length of memory which is then later dereferenced.
>>>
>>> I have worked around the bug by changing the condition on line 213 to
>>> instead be:
>>>
>>>    if(n <= allocated && (n || buf))
>>>
>>> So that if n is zero and buf is null, the else condition is taken.
>>>
>>> Perhaps there is a more elegant solution, but at the least the code I've
>>> changed here is unsafe in that it allows buf to be null on a resize
>>> request to zero length that appears to violate assumptions elsewhere that
>>> the buf is non null.
>>>
>>> Please suggest a more permanent fix or let me know if you feel this
>>> patch is all that is needed.
>>>
>>> Thanks.
>>>
>>> -William
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.randombit.net/pipermail/botan-devel/attachments/20140623/1abcc7b9/attachment.html>


More information about the botan-devel mailing list