[PATCH] Avoid undefined behavior for hashes using XOF

Peter Wu peter at lekensteyn.nl
Fri Mar 25 00:31:10 CET 2016


On Thu, Mar 24, 2016 at 03:34:17PM +0100, Werner Koch wrote:
> 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.

Right, the point I was trying to make is that while we have to avoid the
NULL as second parameter (since it is UB), there are different
conditions to do so.

The above should not be a problem anymore in V2 of this patch as the
function just shortcircuits. Please have a look at
https://lists.gnupg.org/pipermail/gcrypt-devel/2016-March/003770.html

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

The function was defined to "abort on unavailable algorithms"
(presumably cases where gcry_md_test_algo fails), not sure if it is a
good idea to extend the cases where it can crash a program.
(It is unfortunate that no error code can be returned, that makes the
API hard to use (unreliable) in language bindings where the parameters
can be supplied by the user).

> > for XOFs, what about skipping tests for such functions?
> 
> Yes.  But print "skipped".

In v2 (see above) I just omit the test from the generic list of
algorithms (like it skips MD5 in FIPS mode without messages). Only if
you explicitly give it (normally not done) it would try to execute it
(and print garbage).

Is this simple approach sufficient?
-- 
Kind regards,
Peter Wu
https://lekensteyn.nl



More information about the Gcrypt-devel mailing list