Hi Shahriyar
On Fri, 24 Jul 2026 at 13:34, Shahriyar Jalayeri <[email protected]>
wrote:
> The length of a TPM reply is set by the device: tpm_sendrecv_command()
> stores the number of bytes received, caps it only at the command buffer
> size, and reports success whenever the reply's return code is 0. Callers
> then use that length without checking it:
>
> tpm2_get_capability() copies response_len - 15 bytes into the caller's
> buffer. A reply shorter than 15 bytes underflows the subtraction into a
> huge memcpy; a reply longer than the buffer (sized for prop_count
> properties) overruns it.
>
> tpm1_load_key2_oiap() and tpm1_get_pub_key_oiap() pass response_length
> - 41 to verify_response_auth(), which a reply shorter than 41 bytes
> underflows.
>
> A TPM sits on a discrete SPI/I2C/LPC bus that is physically accessible, so
> a reply this malformed is reachable by a bus interposer or a faulty part;
> on the TPM2 path this parsing runs during measured boot.
>
> Check the reported length against the header before the subtraction, and
> against the caller's buffer before the GetCapability copy.
>
> Fixes: 69cd8f0681f4 ("tpm: add TPM2_GetCapability command support")
> Fixes: be6c1529c1ce ("tpm: add AUTH1 cmds for LoadKey2 and GetPubKey")
> Signed-off-by: Shahriyar Jalayeri <[email protected]>
> Acked-by: Miquel Raynal <[email protected]>
> ---
> lib/tpm-v1.c | 6 ++++++
> lib/tpm-v2.c | 8 ++++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/lib/tpm-v1.c b/lib/tpm-v1.c
> index a6727c575fd..5fd22924b28 100644
> --- a/lib/tpm-v1.c
> +++ b/lib/tpm-v1.c
> @@ -757,6 +757,9 @@ u32 tpm1_load_key2_oiap(struct udevice *dev, u32
> parent_handle, const void *key,
> return err;
> }
>
> + if (response_length < TPM_RESPONSE_AUTH_LENGTH)
> + return TPM_LIB_ERROR;
> +
> err = verify_response_auth(0x00000041, response,
> response_length -
> TPM_RESPONSE_AUTH_LENGTH,
> 4, &oiap_session,
> @@ -817,6 +820,9 @@ u32 tpm1_get_pub_key_oiap(struct udevice *dev, u32
> key_handle,
> oiap_session.valid = 0;
> return err;
> }
> + if (response_length < TPM_RESPONSE_AUTH_LENGTH)
> + return TPM_LIB_ERROR;
> +
> err = verify_response_auth(0x00000021, response,
> response_length -
> TPM_RESPONSE_AUTH_LENGTH,
> 0, &oiap_session,
> diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
> index f443b738f82..aa4d3866d7b 100644
> --- a/lib/tpm-v2.c
> +++ b/lib/tpm-v2.c
> @@ -519,6 +519,14 @@ u32 tpm2_get_capability(struct udevice *dev, u32
> capability, u32 property,
> */
> properties_off = sizeof(u16) + sizeof(u32) + sizeof(u32) +
> sizeof(u8) + sizeof(u32);
> + if (response_len < properties_off)
> + return TPM_LIB_ERROR;
> +
> + if (capability == TPM2_CAP_TPM_PROPERTIES &&
> + response_len - properties_off >
> + sizeof(u32) + prop_count * sizeof(struct tpms_tagged_property))
> + return TPM_LIB_ERROR;
> +
>
Thios doesn't llike a problem for the capabilities only. Can't we
update tpm_sendrecv_command() with an extra argument of the expected
response lenght? We can then check that for even invocation in the function
itself if the *size_ptr != NULL
Thanks
/Ilias
> memcpy(buf, &response[properties_off], response_len -
> properties_off);
>
> return 0;
>
> --
> 2.43.0
>
>
