Skip to content

Commit c9a5dff

Browse files
hyperfinitismJuergenReppSIT
authored andcommitted
fix(tpm2_identity_util)!: add buffer size check
hmac_outer_integrity() concatenates two caller-supplied buffers into a fixed-size stack buffer of TPM2_MAX_DIGEST_BUFFER (1024) bytes using memcpy without checking their combined size. This commit adds a buffer size check before memcpy to prevent potential oob. To propagate errors to the caller, the return type of hmac_outer_integrity() and tpm2_identity_util_calculate_outer_integrity() from void to bool. The latter change breaks compatibility with the public API. Signed-off-by: Takuma IMAMURA <209989118+hyperfinitism@users.noreply.github.com>
1 parent a053bdb commit c9a5dff

5 files changed

Lines changed: 28 additions & 8 deletions

File tree

lib/tpm2_identity_util.c

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -285,11 +285,18 @@ static bool aes_encrypt_buffers(TPMT_SYM_DEF_OBJECT *sym,
285285
return result;
286286
}
287287

288-
static void hmac_outer_integrity(TPMI_ALG_HASH parent_name_alg,
288+
static bool hmac_outer_integrity(TPMI_ALG_HASH parent_name_alg,
289289
uint8_t *buffer1, uint16_t buffer1_size, uint8_t *buffer2,
290290
uint16_t buffer2_size, uint8_t *hmac_key,
291291
TPM2B_DIGEST *outer_integrity_hmac) {
292292

293+
if ((size_t)buffer1_size + buffer2_size > TPM2_MAX_DIGEST_BUFFER) {
294+
LOG_ERR("Necessary buffer size (%u) exceeds TPM2_MAX_DIGEST_BUFFER (%zu)",
295+
(unsigned)(buffer1_size + buffer2_size),
296+
(size_t)TPM2_MAX_DIGEST_BUFFER);
297+
return false;
298+
}
299+
293300
uint8_t to_hmac_buffer[TPM2_MAX_DIGEST_BUFFER];
294301
memcpy(to_hmac_buffer, buffer1, buffer1_size);
295302
memcpy(to_hmac_buffer + buffer1_size, buffer2, buffer2_size);
@@ -301,6 +308,7 @@ static void hmac_outer_integrity(TPMI_ALG_HASH parent_name_alg,
301308
to_hmac_buffer, buffer1_size + buffer2_size,
302309
outer_integrity_hmac->buffer, &size);
303310
outer_integrity_hmac->size = size;
311+
return true;
304312
}
305313

306314
bool tpm2_identity_util_calculate_inner_integrity(TPMI_ALG_HASH name_alg,
@@ -376,7 +384,7 @@ bool tpm2_identity_util_calculate_inner_integrity(TPMI_ALG_HASH name_alg,
376384
encrypted_inner_integrity);
377385
}
378386

379-
void tpm2_identity_util_calculate_outer_integrity(TPMI_ALG_HASH parent_name_alg,
387+
bool tpm2_identity_util_calculate_outer_integrity(TPMI_ALG_HASH parent_name_alg,
380388
TPM2B_NAME *pubname, TPM2B_MAX_BUFFER *marshalled_sensitive,
381389
TPM2B_MAX_BUFFER *protection_hmac_key,
382390
TPM2B_MAX_BUFFER *protection_enc_key, TPMT_SYM_DEF_OBJECT *sym_alg,
@@ -390,7 +398,8 @@ void tpm2_identity_util_calculate_outer_integrity(TPMI_ALG_HASH parent_name_alg,
390398
marshalled_sensitive->buffer, marshalled_sensitive->size,
391399
NULL, 0, encrypted_duplicate_sensitive);
392400
//Calculate outerHMAC
393-
hmac_outer_integrity(parent_name_alg, encrypted_duplicate_sensitive->buffer,
401+
return hmac_outer_integrity(parent_name_alg,
402+
encrypted_duplicate_sensitive->buffer,
394403
encrypted_duplicate_sensitive->size, pubname->name, pubname->size,
395404
protection_hmac_key->buffer, outer_hmac);
396405
}

lib/tpm2_identity_util.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ bool tpm2_identity_util_calculate_inner_integrity(TPMI_ALG_HASH name_alg,
9595
* @param outer_hmac
9696
* The outer HMAC structure to populate.
9797
*/
98-
void tpm2_identity_util_calculate_outer_integrity(TPMI_ALG_HASH parent_name_alg,
98+
bool tpm2_identity_util_calculate_outer_integrity(TPMI_ALG_HASH parent_name_alg,
9999
TPM2B_NAME *pubname, TPM2B_MAX_BUFFER *marshalled_sensitive,
100100
TPM2B_MAX_BUFFER *protection_hmac_key,
101101
TPM2B_MAX_BUFFER *protection_enc_key, TPMT_SYM_DEF_OBJECT *sym_alg,

tools/tpm2_duplicate.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,11 +147,14 @@ static tool_rc openssl_create_duplicate(void) {
147147
*/
148148
TPM2B_DIGEST outer_hmac = TPM2B_EMPTY_INIT;
149149
TPM2B_MAX_BUFFER encrypted_duplicate_sensitive = TPM2B_EMPTY_INIT;
150-
tpm2_identity_util_calculate_outer_integrity(
150+
bool outer_res = tpm2_identity_util_calculate_outer_integrity(
151151
ctx.in_parent_public_key_data.publicArea.nameAlg,
152152
&pubname, &marshalled_sensitive, &hmac_key, &enc_key,
153153
&ctx.in_parent_public_key_data.publicArea.parameters.rsaDetail.symmetric,
154154
&encrypted_duplicate_sensitive, &outer_hmac);
155+
if (!outer_res) {
156+
return tool_rc_general_error;
157+
}
155158

156159
/*
157160
* Build the private data structure for writing out

tools/tpm2_import.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,10 +375,15 @@ static tool_rc process_input_ossl_import(ESYS_CONTEXT *ectx) {
375375

376376
TPM2B_DIGEST outer_hmac = TPM2B_EMPTY_INIT;
377377
TPM2B_MAX_BUFFER encrypted_duplicate_sensitive = TPM2B_EMPTY_INIT;
378-
tpm2_identity_util_calculate_outer_integrity(parent_pub->publicArea.nameAlg,
378+
bool outer_res = tpm2_identity_util_calculate_outer_integrity(
379+
parent_pub->publicArea.nameAlg,
379380
&pubname, &encrypted_inner_integrity, &hmac_key, &enc_key,
380381
&parent_pub->publicArea.parameters.rsaDetail.symmetric,
381382
&encrypted_duplicate_sensitive, &outer_hmac);
383+
if (!outer_res) {
384+
rc = tool_rc_general_error;
385+
goto out;
386+
}
382387

383388
result = create_import_key_private_data(parent_pub->publicArea.nameAlg,
384389
&encrypted_duplicate_sensitive, &outer_hmac);

tools/tpm2_makecredential.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,10 +137,13 @@ static tool_rc make_external_credential_and_save(void) {
137137
*/
138138
TPM2B_DIGEST outer_hmac = TPM2B_EMPTY_INIT;
139139
TPM2B_MAX_BUFFER encrypted_sensitive = TPM2B_EMPTY_INIT;
140-
tpm2_identity_util_calculate_outer_integrity(name_alg, &ctx.object_name,
141-
&marshalled_inner_integrity, &hmac_key, &enc_key,
140+
bool outer_res = tpm2_identity_util_calculate_outer_integrity(name_alg,
141+
&ctx.object_name, &marshalled_inner_integrity, &hmac_key, &enc_key,
142142
&ctx.public.publicArea.parameters.rsaDetail.symmetric,
143143
&encrypted_sensitive, &outer_hmac);
144+
if (!outer_res) {
145+
return tool_rc_general_error;
146+
}
144147

145148
/*
146149
* Package up the info to save

0 commit comments

Comments
 (0)