From e9b5cbf992030b99720778bf350a2d9e171e72a8 Mon Sep 17 00:00:00 2001 From: Prasanna Dabi Date: Sat, 16 May 2026 17:47:10 +0530 Subject: [PATCH 1/5] attest/internal: skip SignatureHeaderSize vendor bytes in parseEfiSignatureList 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. --- attest/internal/events.go | 20 ++++++++++-- attest/internal/events_test.go | 57 ++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/attest/internal/events.go b/attest/internal/events.go index 648e88f..fcd6d5f 100644 --- a/attest/internal/events.go +++ b/attest/internal/events.go @@ -381,11 +381,27 @@ func parseEfiSignatureList(b []byte) ([]x509.Certificate, [][]byte, error) { if signatures.Header.SignatureSize > remainingListSize { return nil, nil, fmt.Errorf("SignatureSize %d exceeds remaining signature list space %d", signatures.Header.SignatureSize, remainingListSize) } + // Guard against hash injection via oversized SignatureHeaderSize. + // Per UEFI spec section 31.4.1, SignatureHeaderSize bytes of vendor data + // appear between the fixed header and the actual signature entries. + // SignatureHeaderSize must not consume the entire remaining list space. + if signatures.Header.SignatureHeaderSize >= remainingListSize { + return nil, nil, fmt.Errorf("SignatureHeaderSize %d exceeds remaining signature list space %d", signatures.Header.SignatureHeaderSize, remainingListSize) + } + // Skip the vendor-specific SignatureHeader bytes per UEFI spec section 31.4.1. + // Without this, vendor bytes are misread as signature entries, allowing a + // crafted event log to inject arbitrary hashes into the trusted hash list. + if signatures.Header.SignatureHeaderSize > 0 { + vendorHeader := make([]byte, signatures.Header.SignatureHeaderSize) + if err := binary.Read(buf, binary.LittleEndian, &vendorHeader); err != nil { + return nil, nil, fmt.Errorf("reading signature vendor header: %w", err) + } + } signatureType := signatures.Header.SignatureType switch signatureType { case certX509SigGUID: // X509 certificate - for sigOffset := 0; uint32(sigOffset) < signatures.Header.SignatureListSize-efiSignatureListHeaderSize; { + for sigOffset := int(signatures.Header.SignatureHeaderSize); uint32(sigOffset) < signatures.Header.SignatureListSize-efiSignatureListHeaderSize; { signature := efiSignatureData{} signature.SignatureData = make([]byte, signatures.Header.SignatureSize-efiGUIDSize) err := binary.Read(buf, binary.LittleEndian, &signature.SignatureOwner) @@ -404,7 +420,7 @@ func parseEfiSignatureList(b []byte) ([]x509.Certificate, [][]byte, error) { certificates = append(certificates, *cert) } case hashSHA256SigGUID: // SHA256 - for sigOffset := 0; uint32(sigOffset) < signatures.Header.SignatureListSize-efiSignatureListHeaderSize; { + for sigOffset := int(signatures.Header.SignatureHeaderSize); uint32(sigOffset) < signatures.Header.SignatureListSize-efiSignatureListHeaderSize; { signature := efiSignatureData{} signature.SignatureData = make([]byte, signatures.Header.SignatureSize-efiGUIDSize) err := binary.Read(buf, binary.LittleEndian, &signature.SignatureOwner) diff --git a/attest/internal/events_test.go b/attest/internal/events_test.go index 4a4b2f9..86de46b 100644 --- a/attest/internal/events_test.go +++ b/attest/internal/events_test.go @@ -65,3 +65,60 @@ func TestParseUEFIVariableData(t *testing.T) { t.Errorf("ParseUEFIVariableData() mismatch (-want +got):\n%s", diff) } } + +func TestParseEfiSignatureListNonZeroSignatureHeaderSize(t *testing.T) { + // Regression test: parseEfiSignatureList() must skip SignatureHeaderSize + // vendor bytes before reading signature entries (UEFI spec section 31.4.1). + // + // Without the fix, a crafted EFI_SIGNATURE_LIST with SignatureHeaderSize=48 + // causes attacker-controlled vendor header bytes to be returned as trusted + // SHA256 hashes, allowing arbitrary hash injection into the trusted list. + + // hashSHA256SigGUID bytes (little-endian) + sigType := [16]byte{ + 0x26, 0x16, 0xc4, 0xc1, 0x4c, 0x50, 0x92, 0x40, + 0xac, 0xa9, 0x41, 0xf9, 0x36, 0x93, 0x43, 0x28, + } + + const ( + sigSize = 48 // 16-byte GUID + 32-byte SHA256 + sigHeaderSize = 48 // vendor header exactly one entry worth + ) + + // Vendor header: zero GUID + attacker-chosen hash (0xAA * 32) + attackerHash := bytes.Repeat([]byte{0xAA}, 32) + vendorHeader := make([]byte, sigHeaderSize) + copy(vendorHeader[16:], attackerHash) + + // One legitimate entry: zero GUID + 0xBB * 32 + legitHash := bytes.Repeat([]byte{0xBB}, 32) + legitEntry := make([]byte, sigSize) + copy(legitEntry[16:], legitHash) + + sigListSize := uint32(28 + sigHeaderSize + len(legitEntry)) // 124 bytes + + writeU32 := func(buf *bytes.Buffer, v uint32) { + b := []byte{byte(v), byte(v >> 8), byte(v >> 16), byte(v >> 24)} + buf.Write(b) + } + var buf bytes.Buffer + buf.Write(sigType[:]) + writeU32(&buf, sigListSize) + writeU32(&buf, uint32(sigHeaderSize)) + writeU32(&buf, uint32(sigSize)) + buf.Write(vendorHeader) + buf.Write(legitEntry) + + _, hashes, err := parseEfiSignatureList(buf.Bytes()) + if err != nil { + // Acceptable: the bound check rejects the oversized SignatureHeaderSize. + return + } + + // Attacker hash must NOT appear in the trusted list. + for _, h := range hashes { + if bytes.Equal(h, attackerHash) { + t.Error("attacker-controlled vendor header bytes returned as trusted SHA256 hash") + } + } +} From 1ca9f9b1676bb5ec3ed6ee073393b9fb80689e20 Mon Sep 17 00:00:00 2001 From: Prasanna Dabi Date: Mon, 18 May 2026 14:52:43 +0530 Subject: [PATCH 2/5] attest/internal: address review feedback on SignatureHeaderSize fix - 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 --- attest/internal/events.go | 5 ++--- attest/internal/events_test.go | 39 +++++++++++++++------------------- 2 files changed, 19 insertions(+), 25 deletions(-) diff --git a/attest/internal/events.go b/attest/internal/events.go index fcd6d5f..4a4c17c 100644 --- a/attest/internal/events.go +++ b/attest/internal/events.go @@ -392,9 +392,8 @@ func parseEfiSignatureList(b []byte) ([]x509.Certificate, [][]byte, error) { // Without this, vendor bytes are misread as signature entries, allowing a // crafted event log to inject arbitrary hashes into the trusted hash list. if signatures.Header.SignatureHeaderSize > 0 { - vendorHeader := make([]byte, signatures.Header.SignatureHeaderSize) - if err := binary.Read(buf, binary.LittleEndian, &vendorHeader); err != nil { - return nil, nil, fmt.Errorf("reading signature vendor header: %w", err) + if _, err := buf.Seek(int64(signatures.Header.SignatureHeaderSize), io.SeekCurrent); err != nil { + return nil, nil, fmt.Errorf("seeking past signature vendor header: %w", err) } } diff --git a/attest/internal/events_test.go b/attest/internal/events_test.go index 86de46b..05b2d38 100644 --- a/attest/internal/events_test.go +++ b/attest/internal/events_test.go @@ -81,35 +81,30 @@ func TestParseEfiSignatureListNonZeroSignatureHeaderSize(t *testing.T) { } const ( - sigSize = 48 // 16-byte GUID + 32-byte SHA256 - sigHeaderSize = 48 // vendor header exactly one entry worth + sha256HashSize = 32 // SHA256 digest length in bytes + sigSize = efiGUIDSize + sha256HashSize // 16-byte GUID + 32-byte hash + sigHeaderSize = sigSize // vendor header = one fake entry worth ) - // Vendor header: zero GUID + attacker-chosen hash (0xAA * 32) - attackerHash := bytes.Repeat([]byte{0xAA}, 32) + // Vendor header: zero GUID + attacker-chosen hash (0xAA * sha256HashSize) + attackerHash := bytes.Repeat([]byte{0xAA}, sha256HashSize) vendorHeader := make([]byte, sigHeaderSize) - copy(vendorHeader[16:], attackerHash) + copy(vendorHeader[efiGUIDSize:], attackerHash) - // One legitimate entry: zero GUID + 0xBB * 32 - legitHash := bytes.Repeat([]byte{0xBB}, 32) + // One legitimate entry: zero GUID + 0xBB * sha256HashSize + legitHash := bytes.Repeat([]byte{0xBB}, sha256HashSize) legitEntry := make([]byte, sigSize) - copy(legitEntry[16:], legitHash) + copy(legitEntry[efiGUIDSize:], legitHash) - sigListSize := uint32(28 + sigHeaderSize + len(legitEntry)) // 124 bytes + // Build the EFI_SIGNATURE_LIST using the existing helper, then append + // the vendor header and legitimate entry manually. + // sigListSize = fixed header + vendor header + one entry + sigListSize := uint32(efiSignatureListHeaderSize + sigHeaderSize + len(legitEntry)) + data := buildEFISignatureListData(sigType, sigListSize, sigHeaderSize, sigSize, 0) + data = append(data, vendorHeader...) + data = append(data, legitEntry...) - writeU32 := func(buf *bytes.Buffer, v uint32) { - b := []byte{byte(v), byte(v >> 8), byte(v >> 16), byte(v >> 24)} - buf.Write(b) - } - var buf bytes.Buffer - buf.Write(sigType[:]) - writeU32(&buf, sigListSize) - writeU32(&buf, uint32(sigHeaderSize)) - writeU32(&buf, uint32(sigSize)) - buf.Write(vendorHeader) - buf.Write(legitEntry) - - _, hashes, err := parseEfiSignatureList(buf.Bytes()) + _, hashes, err := parseEfiSignatureList(data) if err != nil { // Acceptable: the bound check rejects the oversized SignatureHeaderSize. return From b6c988571ec584c7f0753cd8b32b5707249102b1 Mon Sep 17 00:00:00 2001 From: Prasanna Dabi Date: Thu, 21 May 2026 09:16:53 +0530 Subject: [PATCH 3/5] attest/internal: fix gofmt formatting --- attest/internal/events_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/attest/internal/events_test.go b/attest/internal/events_test.go index 05b2d38..7b8e0eb 100644 --- a/attest/internal/events_test.go +++ b/attest/internal/events_test.go @@ -81,9 +81,9 @@ func TestParseEfiSignatureListNonZeroSignatureHeaderSize(t *testing.T) { } const ( - sha256HashSize = 32 // SHA256 digest length in bytes + sha256HashSize = 32 // SHA256 digest length in bytes sigSize = efiGUIDSize + sha256HashSize // 16-byte GUID + 32-byte hash - sigHeaderSize = sigSize // vendor header = one fake entry worth + sigHeaderSize = sigSize // vendor header = one fake entry worth ) // Vendor header: zero GUID + attacker-chosen hash (0xAA * sha256HashSize) From af6910468c5202aa4bf6ff8637a5b3dc0187c8b8 Mon Sep 17 00:00:00 2001 From: Prasanna Dabi Date: Thu, 21 May 2026 17:19:56 +0530 Subject: [PATCH 4/5] attest/internal: split SignatureHeaderSize test into two deterministic 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 --- attest/internal/events_test.go | 74 +++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 32 deletions(-) diff --git a/attest/internal/events_test.go b/attest/internal/events_test.go index 7b8e0eb..1b542c4 100644 --- a/attest/internal/events_test.go +++ b/attest/internal/events_test.go @@ -66,54 +66,64 @@ func TestParseUEFIVariableData(t *testing.T) { } } -func TestParseEfiSignatureListNonZeroSignatureHeaderSize(t *testing.T) { - // Regression test: parseEfiSignatureList() must skip SignatureHeaderSize - // vendor bytes before reading signature entries (UEFI spec section 31.4.1). - // - // Without the fix, a crafted EFI_SIGNATURE_LIST with SignatureHeaderSize=48 - // causes attacker-controlled vendor header bytes to be returned as trusted - // SHA256 hashes, allowing arbitrary hash injection into the trusted list. - - // hashSHA256SigGUID bytes (little-endian) +func TestParseEfiSignatureListOversizedSignatureHeaderSize(t *testing.T) { sigType := [16]byte{ 0x26, 0x16, 0xc4, 0xc1, 0x4c, 0x50, 0x92, 0x40, 0xac, 0xa9, 0x41, 0xf9, 0x36, 0x93, 0x43, 0x28, } - const ( - sha256HashSize = 32 // SHA256 digest length in bytes - sigSize = efiGUIDSize + sha256HashSize // 16-byte GUID + 32-byte hash - sigHeaderSize = sigSize // vendor header = one fake entry worth + sha256HashSize = 32 + sigSize = efiGUIDSize + sha256HashSize ) + // sigHeaderSize == remainingListSize: consumes all remaining space. + // The bound check must reject this. + sigHeaderSize := sigSize + sigListSize := uint32(efiSignatureListHeaderSize + sigHeaderSize) + data := buildEFISignatureListData(sigType, sigListSize, uint32(sigHeaderSize), sigSize, 0) + _, _, err := parseEfiSignatureList(data) + if err == nil { + t.Error("parseEfiSignatureList() accepted oversized SignatureHeaderSize, want error") + } +} - // Vendor header: zero GUID + attacker-chosen hash (0xAA * sha256HashSize) - attackerHash := bytes.Repeat([]byte{0xAA}, sha256HashSize) - vendorHeader := make([]byte, sigHeaderSize) - copy(vendorHeader[efiGUIDSize:], attackerHash) - - // One legitimate entry: zero GUID + 0xBB * sha256HashSize +func TestParseEfiSignatureListVendorHeaderNotTrusted(t *testing.T) { + sigType := [16]byte{ + 0x26, 0x16, 0xc4, 0xc1, 0x4c, 0x50, 0x92, 0x40, + 0xac, 0xa9, 0x41, 0xf9, 0x36, 0x93, 0x43, 0x28, + } + const ( + sha256HashSize = 32 + sigSize = efiGUIDSize + sha256HashSize + vendorHeaderSize = sha256HashSize // smaller than sigSize so bound check passes + ) + // Attacker-controlled vendor header bytes. + attackerBytes := bytes.Repeat([]byte{0xAA}, vendorHeaderSize) + // One legitimate entry. legitHash := bytes.Repeat([]byte{0xBB}, sha256HashSize) legitEntry := make([]byte, sigSize) copy(legitEntry[efiGUIDSize:], legitHash) - - // Build the EFI_SIGNATURE_LIST using the existing helper, then append - // the vendor header and legitimate entry manually. - // sigListSize = fixed header + vendor header + one entry - sigListSize := uint32(efiSignatureListHeaderSize + sigHeaderSize + len(legitEntry)) - data := buildEFISignatureListData(sigType, sigListSize, sigHeaderSize, sigSize, 0) - data = append(data, vendorHeader...) + sigListSize := uint32(efiSignatureListHeaderSize + vendorHeaderSize + len(legitEntry)) + data := buildEFISignatureListData(sigType, sigListSize, vendorHeaderSize, sigSize, 0) + data = append(data, attackerBytes...) data = append(data, legitEntry...) - _, hashes, err := parseEfiSignatureList(data) if err != nil { - // Acceptable: the bound check rejects the oversized SignatureHeaderSize. - return + t.Fatalf("parseEfiSignatureList() returned unexpected error: %v", err) } - - // Attacker hash must NOT appear in the trusted list. + // Attacker bytes must NOT appear in the trusted hash list. for _, h := range hashes { - if bytes.Equal(h, attackerHash) { + if bytes.Contains(h, attackerBytes) { t.Error("attacker-controlled vendor header bytes returned as trusted SHA256 hash") } } + // Legitimate hash must be present. + found := false + for _, h := range hashes { + if bytes.Equal(h, legitHash) { + found = true + } + } + if !found { + t.Error("legitimate hash not found in parsed hashes") + } } From 3a4e2b65e0fe7c92ae407a899e8844e689bd7f5d Mon Sep 17 00:00:00 2001 From: Prasanna Dabi Date: Fri, 22 May 2026 10:33:16 +0530 Subject: [PATCH 5/5] attest/internal: simplify TestParseEfiSignatureListVendorHeaderNotTrusted Check length and first element directly instead of iterating, as suggested by reviewer. --- attest/internal/events_test.go | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/attest/internal/events_test.go b/attest/internal/events_test.go index 1b542c4..9dc5eee 100644 --- a/attest/internal/events_test.go +++ b/attest/internal/events_test.go @@ -110,20 +110,10 @@ func TestParseEfiSignatureListVendorHeaderNotTrusted(t *testing.T) { if err != nil { t.Fatalf("parseEfiSignatureList() returned unexpected error: %v", err) } - // Attacker bytes must NOT appear in the trusted hash list. - for _, h := range hashes { - if bytes.Contains(h, attackerBytes) { - t.Error("attacker-controlled vendor header bytes returned as trusted SHA256 hash") - } + if len(hashes) != 1 { + t.Fatalf("parseEfiSignatureList returned %d hashes, expected 1, hashes: %v", len(hashes), hashes) } - // Legitimate hash must be present. - found := false - for _, h := range hashes { - if bytes.Equal(h, legitHash) { - found = true - } - } - if !found { - t.Error("legitimate hash not found in parsed hashes") + if !bytes.Equal(hashes[0], legitHash) { + t.Errorf("parseEfiSignatureList returned hash %x, expected %x", hashes[0], legitHash) } }