The tis_recv
function contains an integer underflow problem that could lead to memory corruption.
size_t tis_recv(struct tpmbuff *buf)
{
u32 expected;
u8 status, *buf_ptr;
struct tpm_header *hdr;
...
hdr = (struct tpm_header *)buf->head;
expected = sizeof(struct tpm_header);
if (recv_data(buf->head, expected) < expected)
goto err;
...
hdr->size = be32_to_cpu(hdr->size);
...
expected = hdr->size - expected;
buf_ptr = tpmb_put(buf, expected);
if (! buf_ptr)
goto err;
if (recv_data(buf_ptr, expected - 1) < (expected - 1))
goto err;
...
Above, the length value that was pulled out of the TPM response header, hdr->size
, could be manipulated by a MITM on the serial bus that connects to the TPM peripheral. If this interposer device sets hdr->size
such that it equals sizeof(struct tpm_header)
, then expected
will be equal to 0
after the subtraction on line 172.
On line 173, tpmb_put
is invoked with expected=0
for its size argument. This allows the length sanitization within the function to pass without error:
u8 *tpmb_put(struct tpmbuff *b, size_t size)
{
u8 *tail = b->tail;
if ((b->len + size) > b->truesize)
return NULL; /* TODO: add overflow buffer support */
b->tail += size;
b->len += size;
return tail;
}
Then finally recv_data
is called with expected-1
as its length argument, which underflows to a very large positive integer 0xFFFF_FFFF
. The interposer can now send more bytes over the bus than will fit in the destination buffer. This of course results in an overflow of buf
.
static size_t recv_data(unsigned char *buf, size_t len)
{
size_t size = 0;
u8 status, *bufptr;
u32 burstcnt = 0;
bufptr = (u8 *)buf;
while (tis_data_available(locality) && size < len) {
burstcnt = burst_wait();
for (; burstcnt > 0 && size < len; burstcnt--) {
*bufptr = tpm_read8(DATA_FIFO(locality));
bufptr++;
size++;
}
}
return size;
}
To prevent this issue, hdr->size
needs to be bounds checked in tis_recv
. It must be at least as large as the TPM response header sizeof(struct tpm_header) + 1
to prevent expected
from underflowing.
Also, there is a comment in tpmb_put
which suggests that an integer overflow check is missing. While that is true, I do not think that an underflow of expected
is exploitable in this function. The value expected
is a u32
type but is cast to size_t
when passed as an argument, and b->len
is size_t
. Therefore, b->len + size
should not be able to wrap, except on 32-bit systems where u32
and size_t
are the same width. But still, checking for integer wrap here is probably a good idea.
Finally, I think the exact same issue is present in the landing-zone repository, here:
https://github.com/TrenchBoot/landing-zone/blob/747bf7a1a6f625559146cf33b9a888403b8cb765/early_tpm.c#L387