liboqs icon indicating copy to clipboard operation
liboqs copied to clipboard

SPHINCS+ memcpy source and destination overlap - undefined behavior

Open bhess opened this issue 3 years ago • 2 comments

The SPHINCS+ implementations have cases of memcpy use where the source and destination overlap. According to the C standard and posix, memcpy behavior is undefined if memory regions overlap.

Detected using valgrind on ppc64le/Ubuntu focal. Memcpy implementations vary, so it seems to be not detected with valgrind on x86_64.

The cause in gen_chain:

https://github.com/open-quantum-safe/liboqs/blob/d9fb4e0e81ccd0a0972b38743404d982fe0a90c1/src/sig/sphincs/pqclean_sphincs-haraka-128f-robust_clean/wots.c#L37-L44

Used for example by wots_gen_pk, where src and dst are the same. The replicated code of all variants is affected:

https://github.com/open-quantum-safe/liboqs/blob/d9fb4e0e81ccd0a0972b38743404d982fe0a90c1/src/sig/sphincs/pqclean_sphincs-haraka-128f-robust_clean/wots.c#L122-L123

Using memmove would be the safe alternative, or avoid memcpy if src and dst are the same.

Below is the valgrind log. It's part of a constant-time check, but the issues detected are because of overlapping memory. ppc64le.txt

bhess avatar Jun 29 '21 07:06 bhess

Yikes -- good catch! Hard to believe the upstreams didn't encounter this before, at least PQClean: Shouldn't this issue then rather be opened there?

baentsch avatar Jun 29 '21 07:06 baentsch

PQClean: Shouldn't this issue then rather be opened there?

Yes, thanks. Created an issue in PQClean that links to the one here.

bhess avatar Jun 29 '21 07:06 bhess

Is this still an issue with the new Sphincs+ code or can this be closed by now?

baentsch avatar Jun 28 '23 09:06 baentsch

I checked and it appears to not be an issue anymore with the current version. Closing.

bhess avatar Jul 10 '23 14:07 bhess