What the developer wrote
Authored by Jon Griffiths
otp: avoid signed left shift
Found by ubsan.
What changed, and why it matters
This commit fixes a subtle programming bug in the code that decodes base32 one-time-password (OTP) secrets. The bug involves shifting a signed integer left, which is undefined behavior in C and was caught by an undefined-behavior sanitizer. The fix changes the variable from a signed integer to an unsigned integer so the shift is well-defined. The practical security impact is likely low, but undefined behavior in cryptographic/OTP handling is generally undesirable because compilers may optimize it unpredictably.
Apply the patch. Consider running ubsan/asan builds regularly on cryptographic and OTP-related code paths, and audit other bit-shift operations for signed/unsigned correctness.
Security signals we found
Undefined behavior in bit-manipulation code
Signed left shift in base32 decoder
Detected by UndefinedBehaviorSanitizer (ubsan)
Code path related to OTP secret decoding
Evidence from the diff
In main/otpauth.c, the base32_to_bin() function used an int tmp = 0; variable to accumulate bits during base32 decoding. Left-shifting a signed int (tmp <<= 5) is undefined behavior in C if the result overflows into the sign bit. The patch changes tmp to unsigned int, making the shift well-defined. The issue was detected by ubsan (UndefinedBehaviorSanitizer). No exploit or incident details are present in the commit or supplied references.
Changed components
main/otpauth.cbase32_to_bin() functionOTP/base32 secret handlingInspect captured patch +1 / −1
diff --git a/main/otpauth.c b/main/otpauth.c
index 066c842..18fb37e 100644
--- a/main/otpauth.c
+++ b/main/otpauth.c
@@ -271,7 +271,7 @@ static bool base32_to_bin(
JADE_ASSERT(b32_dec_len);
JADE_ASSERT(done);
- int tmp = 0;
+ unsigned int tmp = 0;
uint8_t count = 0;
*done = 0;
const char* b32_str_end = b32_str + b32_str_len;
Why this scored 32/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.