Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions attest/internal/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -381,11 +381,26 @@ 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 {
if _, err := buf.Seek(int64(signatures.Header.SignatureHeaderSize), io.SeekCurrent); err != nil {
return nil, nil, fmt.Errorf("seeking past 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)
Expand All @@ -404,7 +419,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)
Expand Down
52 changes: 52 additions & 0 deletions attest/internal/events_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,55 @@ 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 (
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 * sha256HashSize)
attackerHash := bytes.Repeat([]byte{0xAA}, sha256HashSize)
vendorHeader := make([]byte, sigHeaderSize)
copy(vendorHeader[efiGUIDSize:], attackerHash)

// One legitimate entry: zero GUID + 0xBB * sha256HashSize
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...)
data = append(data, legitEntry...)

_, hashes, err := parseEfiSignatureList(data)
if err != nil {
// Acceptable: the bound check rejects the oversized SignatureHeaderSize.
return
Comment thread
prasanna8585 marked this conversation as resolved.
Outdated
}

// 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")
}
}
}
Loading