crypto: STD-compliant shifting in sc_check()
What the developer wrote
Authored by jeffro256
crypto: STD-compliant shifting in sc_check()
What changed, and why it matters
This commit changes a low-level cryptographic helper function in Monero that checks whether a secret scalar number is valid. The original code used direct left bit-shifts on signed integers, which is officially undefined behavior in standard C when the value is negative. The patch replaces those shifts with a helper that either relies on GCC's well-defined behavior or multiplies by a power of two instead. The concern is that undefined behavior in cryptographic code could, in theory, lead to incorrect validation of secret keys, but the commit itself does not describe a concrete exploit or security incident.
Treat as a low-risk hardening patch. Port the signed_lshift helper to any downstream forks still using the old expression. Monitor for follow-up disclosures or CVEs that demonstrate a practical exploit; absent such evidence, no emergency response is warranted.
Security signals we found
Undefined behavior in cryptographic scalar validation
Signed integer left shift replaced with compiler-aware helper
Defensive hardening of sc_check() without claimed exploit
Potential for non-deterministic scalar validation on non-GCC compilers
Evidence from the diff
The function sc_check() in src/crypto/crypto-ops.c validates an Ed25519 scalar against the curve order. It previously computed a weighted sum of signum() results using left shifts (<<) on signed int64_t values. In standard C, left-shifting a negative signed integer is undefined behavior. The patch introduces signed_lshift(), which on GCC keeps the shift (documented as well-defined by GCC) and on other compilers uses multiplication by 1<
Changed components
src/crypto/crypto-ops.csc_check()Ed25519 scalar validationInspect captured patch +19 / −1
diff --git a/src/crypto/crypto-ops.c b/src/crypto/crypto-ops.c
index 314fe44..08556ab 100644
--- a/src/crypto/crypto-ops.c
+++ b/src/crypto/crypto-ops.c
@@ -3810,6 +3810,15 @@ static int64_t signum(int64_t a) {
return a > 0 ? 1 : a < 0 ? -1 : 0;
}
+//! @brief arithmetic left shift for signed operands
+static int64_t signed_lshift(const int64_t a, const int b) {
+#ifdef __GNUC__
+ return a << b; // well-defined in GCC
+#else
+ return a * ((int64_t)1 << b);
+#endif
+}
+
int sc_check(const unsigned char *s) {
int64_t s0 = load_4(s);
int64_t s1 = load_4(s + 4);
@@ -3819,7 +3828,16 @@ int sc_check(const unsigned char *s) {
int64_t s5 = load_4(s + 20);
int64_t s6 = load_4(s + 24);
int64_t s7 = load_4(s + 28);
- return (signum(1559614444 - s0) + (signum(1477600026 - s1) << 1) + (signum(2734136534 - s2) << 2) + (signum(350157278 - s3) << 3) + (signum(-s4) << 4) + (signum(-s5) << 5) + (signum(-s6) << 6) + (signum(268435456 - s7) << 7)) >> 8;
+ return -(0 >
+ ( signum(1559614444 - s0)
+ + signed_lshift(signum(1477600026 - s1), 1)
+ + signed_lshift(signum(2734136534 - s2), 2)
+ + signed_lshift(signum(350157278 - s3), 3)
+ + signed_lshift(signum( - s4), 4)
+ + signed_lshift(signum( - s5), 5)
+ + signed_lshift(signum( - s6), 6)
+ + signed_lshift(signum(268435456 - s7), 7)
+ ));
}
int sc_isnonzero(const unsigned char *s) {
Why this scored 34/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.