What the developer wrote
Authored by tychovrahe
fix(core): fix jpegdec syscall verifier
[no changelog]
What changed, and why it matters
This commit fixes a security weakness in the Trezor hardware wallet's JPEG decoder system call. Before the fix, the kernel did not fully check that the image data pointer and remaining data length were safe for the unprivileged app to access. A malformed request could potentially read memory outside the allowed buffer, which on this secure chip architecture could leak secrets or crash the device. The patch adds explicit bounds checks and confirms the app actually has read permission to the memory region it references.
Treat this as a security fix and include it in the next firmware release. Review other syscall verifiers for similar 'offset + length' validation gaps. Consider whether `probe_read_access(input->data, input->size)` should be replaced entirely by the new offset-aware probe, or keep both for defense in depth. No independent CVE or advisory is supplied, so monitor Trezor's security channels for further guidance.
Security signals we found
syscall verifier missing offset validation
potential out-of-bounds read via unsigned integer underflow (size - offset)
kernel/userspace boundary crossing (syscall wrapper)
memory probe gap between verifier and driver use
fix targets secure-element/embedded firmware syscall surface
Evidence from the diff
The change is in Trezor’s STM32U5 firmware. jpegdec_process__verified() is the syscall verifier wrapper for jpegdec_process(). Previously it only validated input->data and input->size with probe_read_access(input->data, input->size). It did not validate input->offset, and it did not verify that the slice input->data[input->offset .. input->size-1] was readable. The downstream jpegdec_feed_fifo() then casts &inp->data[inp->offset] to uint32_t* and reads up to 16 bytes. If offset > size, the subtraction size - offset underflows, and probe_read_access would be called with a huge length, likely failing but still representing a verifier bypass risk. If offset is within bounds but the verifier only probed from data rather than data+offset, a TOCTOU or short-probe gap could exist. The patch adds if (input->offset > input->size) goto access_violation; and probe_read_access(input->data, input->size - input->offset) before calling the real implementation. A secondary change makes the local pointer const uint32_t* in the driver, which is cosmetic but clarifies read-only intent.
Changed components
core/embed/sys/syscall/stm32/syscall_verifiers.ccore/embed/io/gfx/jpegdec/stm32u5/jpegdec.cjpegdec_process syscallSTM32U5 JPEG decoder driverInspect captured patch +9 / −1
diff --git a/core/embed/io/gfx/jpegdec/stm32u5/jpegdec.c b/core/embed/io/gfx/jpegdec/stm32u5/jpegdec.c
index 8a37259d..b4fa3147 100644
--- a/core/embed/io/gfx/jpegdec/stm32u5/jpegdec.c
+++ b/core/embed/io/gfx/jpegdec/stm32u5/jpegdec.c
@@ -245,7 +245,7 @@ static bool jpegdec_start_dma_transfer(jpegdec_t *dec) {
// Returns `true` if at least one word was written to the FIFO.
static inline bool jpegdec_feed_fifo(jpegdec_t *dec, jpegdec_input_t *inp) {
// Input FIFO needs data
- uint32_t *ptr = (uint32_t *)&inp->data[inp->offset];
+ const uint32_t *ptr = (const uint32_t *)&inp->data[inp->offset];
if (inp->offset + 16 <= inp->size) {
// Feed the FIFO with 16 bytes
JPEG->DIR = ptr[0];
diff --git a/core/embed/sys/syscall/stm32/syscall_verifiers.c b/core/embed/sys/syscall/stm32/syscall_verifiers.c
index ebc76bd0..a8d30581 100644
--- a/core/embed/sys/syscall/stm32/syscall_verifiers.c
+++ b/core/embed/sys/syscall/stm32/syscall_verifiers.c
@@ -1055,6 +1055,14 @@ jpegdec_state_t jpegdec_process__verified(jpegdec_input_t *input) {
goto access_violation;
}
+ if (input->offset > input->size) {
+ goto access_violation;
+ }
+
+ if (!probe_read_access(input->data, input->size - input->offset)) {
+ goto access_violation;
+ }
+
return jpegdec_process(input);
access_violation:
Why this scored 73/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.