boot: bootutil: require exact hash TLV length#2802
Open
d3zd3z wants to merge 1 commit into
Open
Conversation
boot_save_boot_status() only checked that the image hash TLV was no larger than the image_hash buffer, so a TLV shorter than the hash length was accepted and flash_area_read() filled just the first len bytes. The tail of image_hash was then left with whatever happened to be on the stack, and the later memcpy() into the boot record copies sizeof(image_hash) bytes unconditionally, so that stale stack content ends up in the boot record placed in the shared data area. A short hash TLV is malformed, so reject it outright by requiring the exact length, matching the check bootutil_img_validate() already performs. Assisted-by: Claude:opus-5 Signed-off-by: David Brown <david.brown@linaro.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
boot_save_boot_status()reads the image hash out of the manifest into afixed-size stack buffer,
uint8_t image_hash[IMAGE_HASH_SIZE]. The only lengthcheck on the hash TLV was an upper bound:
So a TLV shorter than the configured hash length was accepted, and
flash_area_read()filled only the firstlenbytes. The rest ofimage_hashwas left holding whatever happened to be on the stack.
Further down, the copy into the boot record uses the buffer size rather than
len:so that stale stack content is copied into the record that
boot_add_data_to_shared_area()places in the shared data area, and aconsumer of the record sees it as part of the measurement.
A hash TLV shorter than the hash size is malformed, so this rejects it outright
instead of reading it partially.
bootutil_img_validate()already applies thesame exact-length check (
if (len != sizeof(hash))), so an image that passesvalidation is unaffected by this change; it only tightens the path taken when
the slot is not hash-validated.
Only compiled under
MCUBOOT_MEASURED_BOOT.Testing:
cargo testinsim/passes (25 passed, 0 failed). Note that the simdoes not build
boot/bootutil/src/boot_record.cand never definesMCUBOOT_MEASURED_BOOT, so that is a no-regression run rather than coverage ofthis code; the edited file was additionally compiled standalone with measured
boot enabled, with no diagnostics.