attest/internal: skip SignatureHeaderSize vendor bytes in parseEfiSignatureList#502
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
prasanna8585
changed the title
prasanna8585
force-pushed
the
fix/efi-signature-header-skip
branch
from
ea70dc1 to
b6119d7
Compare
…natureList Per UEFI specification section 31.4.1, an EFI_SIGNATURE_LIST contains SignatureHeaderSize bytes of vendor-specific data between the fixed 28-byte header and the actual signature entries. parseEfiSignatureList() did not advance the buffer past these vendor bytes before reading entries. For hashSHA256SigGUID lists, this allowed attacker-controlled vendor header bytes to be appended to the trusted SHA256 hash list. A crafted TPM event log could inject arbitrary SHA256 hashes into the verifier's trusted measurement database, enabling a remote attestation verifier to accept a compromised boot state. Fix: - Validate: SignatureHeaderSize must not exceed remaining list space - Skip SignatureHeaderSize vendor bytes via binary.Read before entry loops - Fix both certX509SigGUID and hashSHA256SigGUID loop start offsets - Add regression test confirming vendor bytes are not trusted as hashes Reported via Google OSS VRP issue #513787653.
- Use buf.Seek() instead of binary.Read() to skip vendor header bytes (no need to allocate vendorHeader since we do not use its contents) - Add sha256HashSize const instead of magic literal 32 - Reuse buildEFISignatureListData() in regression test - Use efiSignatureListHeaderSize instead of magic literal 28
liamjm
force-pushed
the
fix/efi-signature-header-skip
branch
from
b6119d7 to
1ca9f9b
Compare
|
Hi @liamjm - just following up. The PR has your approval and the CLA is green. Could you please approve the pending CI workflows when you get a chance? Also wanted to flag the related security advisory: GHSA-9r4w-jg96-92mv - happy to answer any questions. |
Please format the code, the linter is complaining.
ACK, I've published this |
…c tests Split TestParseEfiSignatureListNonZeroSignatureHeaderSize into: - TestParseEfiSignatureListOversizedSignatureHeaderSize: verifies the bound check rejects SignatureHeaderSize >= remainingListSize - TestParseEfiSignatureListVendorHeaderNotTrusted: verifies vendor bytes are skipped and do not appear in the trusted hash list
liamjm
left a comment
liamjm
left a comment
There was a problem hiding this comment.
Just a small nit, thanks.
…sted Check length and first element directly instead of iterating, as suggested by reviewer.
|
Thank you @liamjm for the thorough review, patience, and guidance throughout this process. It was a great learning experience contributing to go-attestation. Regarding the CVE - GitHub indicated that Google is the registered CNA for this project and suggested contacting alphabet-cna@google.com. I will reach out to them directly to get a CVE assigned to advisory GHSA-9r4w-jg96-92mv. Please let me know if there is anything else needed from my side. Thank you. |
|
Thanks for your initial finding and the prompt responses.
I work for Google so I can also chase up the advisory.
…On Fri, 22 May 2026, 15:19 Prasanna Dabi, ***@***.***> wrote:
*prasanna8585* left a comment (google/go-attestation#502)
<#502 (comment)>
Thank you @liamjm <https://github.com/liamjm> for the thorough review,
patience, and guidance throughout this process. It was a great learning
experience contributing to go-attestation.
Regarding the CVE - GitHub indicated that Google is the registered CNA for
this project and suggested contacting ***@***.*** I will
reach out to them directly to get a CVE assigned to advisory
GHSA-9r4w-jg96-92mv
<https://github.com/google/go-attestation/security/advisories/GHSA-9r4w-jg96-92mv>.
Please let me know if there is anything else needed from my side.
Thank you.
—
Reply to this email directly, view it on GitHub
<#502 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AARJRVZN3Y3VJZYDWBEJTOD437PMPAVCNFSM6AAAAACZAPLQRSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DKMJVGIZDCOJYGY>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
2 participants
Summary
parseEfiSignatureList()did not skipSignatureHeaderSizevendor bytes before reading signature entries, violating UEFI spec section 31.4.1.Impact
For
hashSHA256SigGUIDlists, attacker-controlled vendor header bytes were appended directly to the trusted SHA256 hash list. A crafted TPM event log could inject arbitrary hashes into the verifier's trusted measurement database, allowing a remote attestation verifier to accept a compromised boot state as legitimate.Fix
SignatureHeaderSizemust not exceed remaining list spaceSignatureHeaderSizevendor bytes viabuf.Seek()before both entry loopscertX509SigGUIDandhashSHA256SigGUIDhandlersTestParseEfiSignatureListNonZeroSignatureHeaderSizeTesting
=== RUN TestParseEfiSignatureListNonZeroSignatureHeaderSize
--- PASS: TestParseEfiSignatureListNonZeroSignatureHeaderSize (0.00s)
PASS
Reported via Google OSS VRP issue #513787653.
Security advisory: GHSA-9r4w-jg96-92mv