[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 21:18:29 EDT 2014


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/875b8241/attachment.html>


More information about the botan-devel mailing list