[PATCH] Avoid undefined behavior for hashes using XOF

[Werner Koch]

On Thu, 24 Mar 2016 13:01, peter at lekensteyn.nl said:

> I think that md_digest_length() is still more suitable here, the message
> is not "avoid memcpy on a NULL source", but "avoid a memcpy if there is
> nothing to copy". The former might hide hypothetical errors where the

The memcpy specs in C99 say

       [#2] The memcpy function copies n characters from the object
       pointed  to  by  s2  into  the  object pointed to by s1.  If
       copying  takes  place  between  objects  that  overlap,  the
       behavior is undefined.

Thus it doesn't mention a NULL pointer.  However, a NULL pointer might
be assumed to trigger the overlapped case and than it would be UB.

> length is non-zero, but md_read returns NULL for some reason.

Not for some reason but because, as you mentioned, SHAKE128 does not
have a read function.  Other hashes may not have one either and thus I
think that checking for the NULL case is more robust that checking for
the algo's length.  At worst a wrong definition of a hash (or CRC) could
give a length but no read function.

> Failing with an error indeed seems a better hint for the user, I will
> fix that for the next patch. The benchmark is not quite accurate anyway

Right, gcry_md_hash_buffers used to be simple function to return a SHA-1
hash and that was always defined.  However, I latter changed that to
allow all algos but I could not add an error return because existing
callers do not check it.  That was fixed with hash_buffers.

You may also just die if md_read() return NULL.  This is what
gcry_md_hash_buffers does for inknown algorithms.

> for XOFs, what about skipping tests for such functions?

Yes.  But print "skipped".


Salam-Shalom,

   Werner


-- 
Die Gedanken sind frei.  Ausnahmen regelt ein Bundesgesetz.