Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upparseRawEvent can allocate an unbounded amount of memory #126
Comments
This comment has been minimized.
This comment has been minimized.
|
Nice catch :) What about adding a check to ensure h.EventSize isn't bigger than the remaining bytes in the measurement log? func parseRawEvent(r *bytes.Buffer) (event rawEvent, err error) {
var h rawEventHeader
if err = binary.Read(r, binary.LittleEndian, &h); err != nil {
return event, err
}
if h.EventSize > uint32(r.Len()) {
return event, fmt.Errorf("event size (%d) was greater than remaining bytes in measurement log (%d)", h.EventSize, r.Len())
}
data := make([]byte, int(h.EventSize))
if _, err := io.ReadFull(r, data); err != nil {
return event, err
}
return rawEvent{
typ: EventType(h.Type),
data: data,
index: int(h.PCRIndex),
digests: [][]byte{h.Digest[:]},
}, nil
} |
This comment has been minimized.
This comment has been minimized.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In this code path an attacker can control the number of allocated bytes. This can lead to a DoS attack by OOMing the process.
Example:
Produces:
rawEventHeader{ PCRIndex:0x30343933, Type:0x36303032, Digest:[20]uint8{0x31, 0x39, 0x36, 0x33, 0x39, 0x34, 0x34, 0x37, 0x39, 0x32, 0x31, 0x32, 0x32, 0x37, 0x39, 0x30, 0x34, 0x30, 0x31, 0x6d}, EventSize:0xbdbfef47, }0xbdbfef47being 3.183 GB.It doesn't appear that the TCG EFI Protocol Specification defines a maximum size for an event. So it seems our options are either choosing an arbitrary maximum or reporting this to the TCG as undefined behavior.