SPHINCS+ memcpy source and destination overlap - undefined behavior

[bhess]

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:

liboqs/src/sig/sphincs/pqclean_sphincs-haraka-128f-robust_clean/wots.c

Lines 37 to 44 in d9fb4e0

	static void gen_chain(unsigned char *out, const unsigned char *in,
	unsigned int start, unsigned int steps,
	const unsigned char *pub_seed, uint32_t addr[8],
	const hash_state *hash_state_seeded) {
	uint32_t i;
	/* Initialize out with the value at position 'start'. */
	memcpy(out, in, PQCLEAN_SPHINCSHARAKA128FROBUST_CLEAN_N);

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

liboqs/src/sig/sphincs/pqclean_sphincs-haraka-128f-robust_clean/wots.c

Lines 122 to 123 in d9fb4e0

	gen_chain(pk + i * PQCLEAN_SPHINCSHARAKA128FROBUST_CLEAN_N, pk + i * PQCLEAN_SPHINCSHARAKA128FROBUST_CLEAN_N,
	0, PQCLEAN_SPHINCSHARAKA128FROBUST_CLEAN_WOTS_W - 1, pub_seed, addr, hash_state_seeded);

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

[baentsch]

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?

[bhess]

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

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

[baentsch]

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

[bhess]

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