fix(crypto): add missing checks for negative VLA size
What the developer wrote
Authored by M1nd3r
fix(crypto): add missing checks for negative VLA size
Credit for finding this issue belongs to a security reporter Amr.
[no changelog]
What changed, and why it matters
This commit fixes two functions in Trezor's base58 code that build temporary memory buffers using a user-supplied length. Before the fix, a negative length value was not rejected. On systems where negative signed integers are treated as very large unsigned values, that could cause the device to allocate a huge buffer or corrupt memory, potentially crashing the device or enabling further attacks. The fix adds simple checks to reject negative lengths.
Review all callers of base58_encode_check and base58_decode_check to ensure they cannot pass negative lengths, and audit other VLA usages in the codebase for similar missing lower-bound checks. Consider replacing VLAs with fixed-size or heap-allocated buffers where feasible.
Security signals we found
Missing negative-length validation before variable-length array allocation
Potential stack-based buffer overflow or allocation failure from signed/unsigned conversion
External security reporter credited (Amr)
Evidence from the diff
In crypto/base58.c, base58_encode_check and base58_decode_check declare variable-length arrays (VLAs) using int datalen: uint8_t buf[datalen + 32] and uint8_t d[datalen + 4]. The prior bounds check only rejected datalen > 128, so a negative datalen would pass. Depending on calling conventions and integer conversion rules, a negative int used as a VLA size is undefined behavior; in practice it can be interpreted as a large size_t, leading to stack exhaustion or memory corruption. The patch adds datalen < 0 to the guard, closing the path.
Changed components
crypto/base58.cbase58_encode_checkbase58_decode_checkInspect captured patch +2 / −2
diff --git a/crypto/base58.c b/crypto/base58.c
index ad8c78cd..2b7b36fd 100644
--- a/crypto/base58.c
+++ b/crypto/base58.c
@@ -187,7 +187,7 @@ bool b58enc(char *b58, size_t *b58sz, const void *data, size_t binsz) {
int base58_encode_check(const uint8_t *data, int datalen,
HasherType hasher_type, char *str, int strsize) {
- if (datalen > 128) {
+ if (datalen < 0 || datalen > 128) {
return 0;
}
uint8_t buf[datalen + 32];
@@ -203,7 +203,7 @@ int base58_encode_check(const uint8_t *data, int datalen,
int base58_decode_check(const char *str, HasherType hasher_type, uint8_t *data,
int datalen) {
- if (datalen > 128) {
+ if (datalen < 0 || datalen > 128) {
return 0;
}
uint8_t d[datalen + 4];
Why this scored 59/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.