fix(core): add missing buffer length checks
What the developer wrote
Authored by cepetr
fix(core): add missing buffer length checks
[no changelog]
What changed, and why it matters
This commit adds several missing safety checks in low-level hardware code for the Trezor hardware wallet. It prevents reading past the end of a too-short firmware image, avoids odd behavior when asked to read or write zero SD card blocks, ensures a signature buffer is large enough before use, and fixes a debug-message routine that could truncate or mishandle strings. These are defensive fixes; the commit message does not call them security fixes, but missing length checks in embedded code can sometimes be exploited to crash the device or leak nearby memory.
Treat as a routine hardening patch. Review whether the too-short image path, zero-block SD path, and small signature buffer were reachable from user-facing workflows, and confirm the debug console is disabled in production builds. No immediate incident response is indicated by the diff alone.
Security signals we found
Missing buffer length check added before parsing image header
Zero-length SD card block operation now handled explicitly
Signature output buffer size validated before DER encoding offset
Debug string copy changed from strncpy to bounded memcpy with consistent buffer size
No changelog entry and no vendor security disclosure supplied
Evidence from the diff
The patch adds length/edge-case checks across six embedded files: (1) nrf_update.c rejects images smaller than struct image_header before parsing; (2) sdcard.c read/write paths on stm32f4 and stm32u5 return sectrue early for num_blocks == 0, avoiding possible zero-length DMA or command issues; (3) optiga.c verifies max_der_signature_size >= 2 before writing the DER signature at offset 2; (4) dbg_console_backend.c replaces a fixed strncpy into a 512-byte static buffer with a size-capped memcpy, uses SEGGER_SYSVIEW_MAX_STRING_LEN + 1 for the buffer, and reduces SEGGER_SYSVIEW_MAX_STRING_LEN from 1024 to 512 so the buffer size matches the macro. No changelog entry is present and no security advisory or CVE is referenced.
Changed components
core/embed/io/nrf/stm32u5/nrf_update.ccore/embed/io/sdcard/stm32f4/sdcard.ccore/embed/io/sdcard/stm32u5/sdcard.ccore/embed/sec/optiga/optiga.ccore/embed/sys/dbg/stm32/dbg_console_backend.ccore/embed/sys/dbg/stm32/systemview/config/SEGGER_SYSVIEW_Conf.hInspect captured patch +34 / −10
### core/embed/io/nrf/stm32u5/nrf_update.c
@@ -169,6 +169,10 @@ static int version_cmp(const nrf_app_version_t *v1,
}
bool nrf_update_required(const uint8_t *image_ptr, size_t image_len) {
+ if (image_len < sizeof(struct image_header)) {
+ return false;
+ }
+
for (int i = 0; i < 3; i++) {
nrf_info_t info;
uint8_t expected_hash[SHA256_DIGEST_LENGTH];
### core/embed/io/sdcard/stm32f4/sdcard.c
@@ -293,6 +293,10 @@ secbool sdcard_read_blocks(uint32_t *dest, uint32_t block_num,
return secfalse;
}
+ if (num_blocks == 0) {
+ return sectrue;
+ }
+
// check that dest pointer is aligned on a 4-byte boundary
if (((uint32_t)dest & 3) != 0) {
return secfalse;
@@ -349,6 +353,10 @@ secbool sdcard_write_blocks(const uint32_t *src, uint32_t block_num,
return secfalse;
}
+ if (num_blocks == 0) {
+ return sectrue;
+ }
+
// check that src pointer is aligned on a 4-byte boundary
if (((uint32_t)src & 3) != 0) {
return secfalse;
### core/embed/io/sdcard/stm32u5/sdcard.c
@@ -296,6 +296,10 @@ secbool sdcard_read_blocks(uint32_t *dest, uint32_t block_num,
return secfalse;
}
+ if (num_blocks == 0) {
+ return sectrue;
+ }
+
// check that dest pointer is aligned on a 4-byte boundary
if (((uint32_t)dest & 3) != 0) {
return secfalse;
@@ -320,6 +324,10 @@ secbool sdcard_write_blocks(const uint32_t *src, uint32_t block_num,
return secfalse;
}
+ if (num_blocks == 0) {
+ return sectrue;
+ }
+
// check that src pointer is aligned on a 4-byte boundary
if (((uint32_t)src & 3) != 0) {
return secfalse;
### core/embed/sec/optiga/optiga.c
@@ -139,6 +139,11 @@ optiga_sign_result optiga_sign(uint8_t index, const uint8_t *digest,
}
#endif // SECRET_KEY_MASKING
+ if (max_der_signature_size < 2) {
+ ret = OPTIGA_SIGN_ERROR;
+ goto cleanup;
+ }
+
optiga_result res = optiga_calc_sign(
OPTIGA_OID_ECC_KEY + index, digest, digest_size, &der_signature[2],
max_der_signature_size - 2, der_signature_size);
### core/embed/sys/dbg/stm32/dbg_console_backend.c
@@ -60,16 +60,15 @@ static ssize_t itm_swo_write(const void *data, size_t data_size) {
#ifdef USE_DBG_CONSOLE_SYSTEM_VIEW
static ssize_t sysview_write(const void *data, size_t data_size) {
-#if 1
- static char str[512];
- strncpy(str, (const char *)data, sizeof(str) - 1);
- str[sizeof(str) - 1] = 0;
+ static char str[SEGGER_SYSVIEW_MAX_STRING_LEN + 1];
+
+ size_t copy_size = MIN(data_size, sizeof(str) - 1);
+ memcpy(str, data, copy_size);
+ str[copy_size] = 0;
+
SEGGER_SYSVIEW_Print(str);
-#endif
-#if 0
- SEGGER_RTT_Write(0, data, data_size);
-#endif
- return MIN(data_size, sizeof(str) - 1);
+
+ return copy_size;
}
#endif
### core/embed/sys/dbg/stm32/systemview/config/SEGGER_SYSVIEW_Conf.h
@@ -77,7 +77,7 @@ Additional information:
#define SEGGER_SYSVIEW_RTT_BUFFER_SIZE 4096
-#define SEGGER_SYSVIEW_MAX_STRING_LEN 1024
+#define SEGGER_SYSVIEW_MAX_STRING_LEN 512
#define SEGGER_SYSVIEW_BUFFER_SECTION "CCMRAM"
Why this scored 51/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.