fix(core): fix wrong type of returned value
What the developer wrote
Authored by cepetr
fix(core): fix wrong type of returned value
[no changelog]
What changed, and why it matters
A function that reads firmware image headers was returning the wrong kind of error value. In C, returning a special 'false' value where a pointer is expected can confuse the rest of the program, potentially causing crashes or allowing a malformed firmware image to be treated as valid. The patch makes all error paths consistently return NULL, which is the normal 'no valid header' indicator for this function.
Review all callers of read_image_header() to confirm they check for NULL and do not rely on the previous secfalse behavior. Consider whether any reachable code path could have treated secfalse as a valid pointer. Add a regression test or static-analysis rule to prevent mixed-type returns.
Security signals we found
Type confusion between pointer and boolean return values
Inconsistent error handling in image header validation
Potential bypass of firmware image sanity checks if callers misinterpret return value
No changelog or security disclosure in commit message
Evidence from the diff
read_image_header() in core/embed/util/image/image.c returns a const image_header pointer on success. Four validation failure paths were returning secfalse, an int/bool-style sentinel, instead of NULL. Because the return type is a pointer, callers comparing the result against NULL or treating it as a pointer could misinterpret secfalse as a valid low-address pointer or otherwise mishandle the error. The patch changes those four returns to NULL so the function contract is consistent.
Changed components
core/embed/util/image/image.cread_image_header()Trezor Core firmware image loading/validationInspect captured patch +4 / −4
diff --git a/core/embed/util/image/image.c b/core/embed/util/image/image.c
index 08ebde17d..c417e596d 100644
--- a/core/embed/util/image/image.c
+++ b/core/embed/util/image/image.c
@@ -111,11 +111,11 @@ const image_header *read_image_header(const uint8_t *const data,
// lowest bit is used for breaking compatibility between old TT bootloaders
// and non TT images
// which is evaluated in check_image_model function
- if ((hdr->expiry & 0xFFFFFFFE) != 0) return secfalse;
+ if ((hdr->expiry & 0xFFFFFFFE) != 0) return NULL;
- if (hdr->codelen > (maxsize - hdr->hdrlen)) return secfalse;
- if ((hdr->hdrlen + hdr->codelen) < 4 * 1024) return secfalse;
- if ((hdr->hdrlen + hdr->codelen) % 512 != 0) return secfalse;
+ if (hdr->codelen > (maxsize - hdr->hdrlen)) return NULL;
+ if ((hdr->hdrlen + hdr->codelen) < 4 * 1024) return NULL;
+ if ((hdr->hdrlen + hdr->codelen) % 512 != 0) return NULL;
return hdr;
}
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.