So calling fe25519_abs(x, x); triggers a call to memcpy(x, x, ...), which is undefined behavior in C. Indeed, the C99 standard defined in section "7.21.2.1 The memcpy function":
Calling memcpy with s1 == s2 violates the assertions ensured by restrict.
To fix this, I believe memmove should be used instead of memcpy in fe25519_copy.
For information, this issue was found by running clang's static analyzer with scan-build -analyze-headers -enable-checker alpha.unix.cstring.BufferOverlap make. It reported:
./include/sodium/private/ed25519_ref10_fe_51.h:194:5: warning: Arguments must not be overlapping buffers [alpha.unix.cstring.BufferOverlap]
memcpy(h, f, 5 * sizeof h[0]);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Hello,
In current git
master
, several functions are calling functionfe25519_abs
with twice the same pointer. For example:ristretto255_sqrt_ratio_m1
: https://github.com/jedisct1/libsodium/blob/7978205916784fea4db9f8f540be7bd87043425f/src/libsodium/crypto_core/ed25519/ref10/ed25519_ref10.c#L2894ristretto255_frombytes
: https://github.com/jedisct1/libsodium/blob/7978205916784fea4db9f8f540be7bd87043425f/src/libsodium/crypto_core/ed25519/ref10/ed25519_ref10.c#L2959ristretto255_p3_tobytes
: https://github.com/jedisct1/libsodium/blob/7978205916784fea4db9f8f540be7bd87043425f/src/libsodium/crypto_core/ed25519/ref10/ed25519_ref10.c#L3022ristretto255_elligator
: https://github.com/jedisct1/libsodium/blob/7978205916784fea4db9f8f540be7bd87043425f/src/libsodium/crypto_core/ed25519/ref10/ed25519_ref10.c#L3054This call as the consequence of calling
memcpy
with the same pointer too:fe25519_abs
callsfe25519_cneg
with the same pointers: https://github.com/jedisct1/libsodium/blob/7978205916784fea4db9f8f540be7bd87043425f/src/libsodium/crypto_core/ed25519/ref10/ed25519_ref10.c#L185-L189fe25519_cneg
callsfe25519_copy
with the same pointers: https://github.com/jedisct1/libsodium/blob/7978205916784fea4db9f8f540be7bd87043425f/src/libsodium/crypto_core/ed25519/ref10/ed25519_ref10.c#L175-L183And
fe25519_copy
is a wrapper aroundmemcpy
: https://github.com/jedisct1/libsodium/blob/7978205916784fea4db9f8f540be7bd87043425f/src/libsodium/include/sodium/private/ed25519_ref10_fe_51.h#L231-L235 https://github.com/jedisct1/libsodium/blob/7978205916784fea4db9f8f540be7bd87043425f/src/libsodium/include/sodium/private/ed25519_ref10_fe_25_5.h#L287-L291So calling
fe25519_abs(x, x);
triggers a call tomemcpy(x, x, ...)
, which is undefined behavior in C. Indeed, the C99 standard defined in section "7.21.2.1 The memcpy function":Calling
memcpy
withs1 == s2
violates the assertions ensured byrestrict
.To fix this, I believe
memmove
should be used instead ofmemcpy
infe25519_copy
.For information, this issue was found by running clang's static analyzer with
scan-build -analyze-headers -enable-checker alpha.unix.cstring.BufferOverlap make
. It reported: