From 8081e2b0183053d3d1e6f3c12ca71f485bccac07 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Wed, 4 Dec 2024 23:04:35 +0800 Subject: [PATCH] Avoid curve25519 "left shift of negative value" Cast to unsigned before performing the left shift. The shifted result is then sign extended back before subtraction, giving the same output. Generated assembly is unaltered with this change, checking gcc 14.2.0 and clang 19.1.1 (x86-64). The same unsigned cast fix is present in crypto++'s tweetnacl version. https://github.com/weidai11/cryptopp/pull/566/commits/5be0c0a3f9fe6e8cb64cc27d77706eeb6e4a7d0f#diff-067e6f20b212f32eb7fcdec3395fb1d145612f5d44736d041c1dfc9bc44a9d5b Jeffery Walton's modification to modL() is also applied here. Running with -fsanitize=undefined could report a left shift of a negative value from car25519 o[i]-=c<<16; This is valid for gcc (and presumably clang), but the sanitizer report is a problem. https://gcc.gnu.org/onlinedocs/gcc-14.2.0/gcc/Integers-implementation.html Fixes #312 on github --- src/curve25519.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/curve25519.c b/src/curve25519.c index 51e0e76ec..5f743e358 100644 --- a/src/curve25519.c +++ b/src/curve25519.c @@ -88,7 +88,7 @@ sv car25519(gf o) o[i]+=(1LL<<16); c=o[i]>>16; o[(i+1)*(i<15)]+=c-1+37*(c-1)*(i==15); - o[i]-=c<<16; + o[i]-=((u64)c)<<16; } } @@ -363,7 +363,7 @@ sv modL(u8 *r,i64 x[64]) for (j = i - 32;j < i - 12;++j) { x[j] += carry - 16 * x[i] * L[j - (i - 32)]; carry = (x[j] + 128) >> 8; - x[j] -= carry << 8; + x[j] -= ((u64)carry) << 8; } x[j] += carry; x[i] = 0;