fix(image): pin the whole .efw header, not four fields of it - #87
Open
Kartikey1306 wants to merge 1 commit into
Open
fix(image): pin the whole .efw header, not four fields of it#87Kartikey1306 wants to merge 1 commit into
Kartikey1306 wants to merge 1 commit into
Conversation
Closes embeddedos-org#66 (parts 1 and 2). eos_image_header_t is a wire format — eFirmware writes these bytes, eBoot reads them, and the signing tools address fields by absolute offset. Four of its fourteen fields were pinned: sizeof, hash, sig_type, signature. Transposing two adjacent same-width fields moves neither sizeof nor any of those four offsets. With load_addr and entry_addr swapped, all four existing asserts still pass and the header compiles clean; the bootloader then loads an image at its entry point and jumps to its load address. No constant's *value* was pinned on this side either. EOS_IMG_MAGIC, EOS_HASH_SIZE, EOS_SIG_MAX_SIZE, EOS_IMG_SIGNED_LEN, EOS_IMAGE_HDR_ VERSION and the five eos_sig_type_t values all travel inside the image. Changing EOS_SIG_ED25519 from 3 to 4 compiled cleanly, passed every assert, and would make eBoot misread the signature type of every image already in the field. eFirmware pins EFW_IMAGE_MAGIC and EFW_SIG_ED25519 on its side; this is the matching half, so the two definitions can no longer drift apart in silence. Adds the ten missing offsets, three field widths (an offset assert cannot see a field growing into padding that keeps every later offset — reserved[] absorbs exactly that), and eleven constant values. Thirty asserts total. Header only. No struct member, constant, or line of logic changes, so every image on disk today parses exactly as before. Verified: compiles clean, all 30 pass load_addr/entry_addr transposed -> 2 asserts fire (master: 0) EOS_SIG_ED25519 = 3 -> 4 -> 1 assert fires (master: 0) cmake -DEBLDR_BUILD_TESTS=ON + ctest -> 20/20 passed Part 3 of embeddedos-org#66 — eFirmware stamping v1 while eBoot supports only v2 — is a security-policy call and is deliberately not touched here.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Closes #66, parts 1 and 2.
eos_image_header_tis a wire format: eFirmware writes these bytes, eBoot reads them, and the signing tools address fields by absolute offset. Four of its fourteen fields were pinned —sizeof,hash,sig_type,signature.A field swap passed every existing assert
Transposing two adjacent same-width fields moves neither
sizeofnor any of those four offsets:The bootloader would then load an image at its entry point and jump to its load address. With this change:
No constant's value was pinned on this side
EOS_IMG_MAGIC,EOS_HASH_SIZE,EOS_SIG_MAX_SIZE,EOS_IMG_SIGNED_LEN,EOS_IMAGE_HDR_VERSIONand the fiveeos_sig_type_tvalues all travel inside the image, so they are wire format too, and an offset assert says nothing about them.That change would make eBoot misread the signature type of every image already in the field. eFirmware pins
EFW_IMAGE_MAGIC == 0x454F5349uandEFW_SIG_ED25519 == 3on its side; these are the matching half, so the two definitions can no longer drift apart in silence.What is added
The three width asserts are there because an offset assert cannot see a field growing into padding that happens to keep every later offset —
reserved[30]absorbs exactly that.Header only. No struct member, constant, or line of logic changes, so every image on disk today parses exactly as it did before. The asserts are all
EOS_IMG_STATIC_ASSERT, which already degrades to nothing before C11.Validation
Not touched
Part 3 of #66 — eFirmware stamping
hdr_version = 1while eBoot supports only v2, socore/image_verify.cadmits a v1 image at the parser and then fails it at the signature — is a security-policy call (reject at parse, branch the verifier, or change what efwtool stamps). It fails closed, so it is not an escalation, and it wants a decision rather than a patch. Deliberately left out.🤖 Generated with Claude Code