[PATCH] Avoid undefined behavior for hashes using XOF
wk at gnupg.org
Thu Mar 24 15:34:17 CET 2016
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".
Die Gedanken sind frei. Ausnahmen regelt ein Bundesgesetz.
More information about the Gcrypt-devel