[PATCH 1/2] tpm: sessions: bound the primary public area by the response size

[PATCH 1/2] tpm: sessions: bound the primary public area by the response size

[Guo Zihao]

tpm2_parse_create_primary() reads the size of the TPM2B_PUBLIC area
from the response and hashes it to compute the object name:

	len = tpm_buf_read_u16(buf, &offset_r);
	offset_t = offset_r;
	if (name) {
		/*
		 * now we have the public area, compute the name of
		 * the object
		 */
		put_unaligned_be16(TPM_ALG_SHA256, name);
		sha256(&buf->data[offset_r], len, name + 2);
	}

The length is only checked against the response later, after the
hash has been taken. A TPM that reports a public area longer than
what it actually sent makes the sha256() run past the end of the
received data and hash whatever follows the response in the buffer.

Check that the public area fits in the response before hashing it.

Fixes: d2add27cf2b8 ("tpm: Add NULL primary creation")
Cc: stable@vger.kernel.org
Signed-off-by: Guo Zihao <guozh23@xiaopeng.com>
---
 drivers/char/tpm/tpm2-sessions.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index ca1e2bf42..3524c1d50 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -1126,6 +1126,8 @@ static int tpm2_parse_create_primary(struct tpm_chip *chip, struct tpm_buf *buf,
 		return -EINVAL;
 	len = tpm_buf_read_u16(buf, &offset_r);
 	offset_t = offset_r;
+	if (offset_r + len > tpm_buf_length(buf))
+		return -EINVAL;
 	if (name) {
 		/*
 		 * now we have the public area, compute the name of
-- 
2.50.1

[PATCH 2/2] tpm: sessions: bound the parameter decryption by the response size

[Guo Zihao]

tpm_buf_check_hmac_response() decrypts the parameter area of a
response for sessions with TPM2_SA_ENCRYPT set:

	len = tpm_buf_read_u16(buf, &offset_p);
	aes_prepareenckey(&auth->aes_key, auth->scratch, AES_KEY_BYTES);
	aescfb_decrypt(&auth->aes_key, &buf->data[offset_p],
		       &buf->data[offset_p], len,
		       auth->scratch + AES_KEY_BYTES);

The length is read from the response itself and never checked
against the buffer: every other size taken from a session response
is compared to tpm_buf_length(), but not this one. A TPM that
reports an oversized parameter area makes aescfb_decrypt() walk
past the end of the buffer, and since the decryption is done in
place, this both reads and writes whatever follows the received
data.

Check that the encrypted parameters fit in the response before
decrypting them.

Fixes: 1085b8276bb4 ("tpm: Add the rest of the session HMAC API")
Cc: stable@vger.kernel.org
Signed-off-by: Guo Zihao <guozh23@xiaopeng.com>
---
 drivers/char/tpm/tpm2-sessions.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 3524c1d50..6245a6baa 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -872,6 +872,10 @@ int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
 			  auth->scratch);
 
 		len = tpm_buf_read_u16(buf, &offset_p);
+		if (offset_p + len > tpm_buf_length(buf)) {
+			rc = -EINVAL;
+			goto out;
+		}
 		aes_prepareenckey(&auth->aes_key, auth->scratch, AES_KEY_BYTES);
 		aescfb_decrypt(&auth->aes_key, &buf->data[offset_p],
 			       &buf->data[offset_p], len,
-- 
2.50.1