[PATCH] Avoid undefined behavior for hashes using XOF

[Werner Koch]

On Thu, 24 Mar 2016 00:29, peter at lekensteyn.nl said:

> While the functions could simply shortcircuit and return early, let's
> perform the hash calculations anyway such that the benchmarks can be
> run. Copying zero bytes is valid according to the documentation of
> gcry_md_hash_buffer{,s} as gcry_md_get_algo_dlen() returns 0.

Your code is now:

  if (md_digest_length (algo))
    memcpy (digest, md_read (h, algo), md_digest_length (algo));

By adding the condition you avoid calling md_read which would return
NULL in the case of SHAKE128.  So the UB seems to be that memcpy (foo,
NULL, 0) is not defined - impractical but obviously another gcc/clang
annoyance.

I would suggest not to test for md_digest_length but to

  const void *tmp = md_read (h, algo);
  if (tmp)
    memcpy (digest, tmp, md_digest_length (algo));

which uses the real cause for the condition.  

_gcry_md_hash_buffers should however return an error and not silently
ignore it.  Even if that means to adjust the tests ;-)


Salam-Shalom,

   Werner

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