Skip to content

tpm2-provider-cipher: Fix potential allocator–deallocator inconsistency for IV buffers#163

Closed
hyperfinitism wants to merge 1 commit intotpm2-software:masterfrom
hyperfinitism:fix/ivector-handling
Closed

tpm2-provider-cipher: Fix potential allocator–deallocator inconsistency for IV buffers#163
hyperfinitism wants to merge 1 commit intotpm2-software:masterfrom
hyperfinitism:fix/ivector-handling

Conversation

@hyperfinitism
Copy link
Copy Markdown
Contributor

Issue

In tpm2-provider-cipher.c, cctx->ivector is initially allocated using OPENSSL_zalloc and finally deallocated using OPENSSL_clear_free().

In the tpm2_cipher_process_buffer/update_stream() functions, the IV buffer cctx->ivector will be replaced with the IV buffer ivector allocated in encrypt_decrypt(); this may happen allocator–deallocator inconsistency.

tpm2-tss internally uses the standard malloc/free(), while OpenSSL uses the OPENSSL_malloc/free(); the latter may internally use custom allocator/reallocator/deallocator set via CRYPTO_set_mem_functions().

Fix

This commit resolves this potential inconsistency as follows:

  • Before
    1. cleanse and free cctx->ivector
    2. cctx->ivector = ivector
  • After
    1. memcpy ivector to cctx->ivector
    2. cleanse and free ivector

Test

I have confirmed that make check completed successfully.

In tpm2-provider-cipher, cctx->ivector is allocated using OPENSSL_zalloc
and deallocated using OPENSSL_clear_free().

During tpm2_cipher_process_buffer/update_stream(), the IV buffer
cctx->ivector will be replaced with the IV buffer ivector allocated in
encrypt_decrypt(); this may happen (de)allocator inconsistency.

tpm2-tss uses the standard malloc/free(), while OpenSSL uses the
OPENSSL_malloc/free(); the latter may use custom (de)allocator set via
CRYPTO_set_mem_functions().

This commit resolves this potential malloc/free inconsistency/

Signed-off-by: Takuma IMAMURA <209989118+hyperfinitism@users.noreply.github.com>
@gotthardp
Copy link
Copy Markdown
Contributor

Thank you very much for your contribution. Also in relation to #164 I prefer to have an equivalent for OPENSSL_clear_free, so I created cleanse_free and used it to fix this issue.

If you agree, I suggest to cancel this PR and use the PR #168 instead. I added you there as the co-author.

Copy link
Copy Markdown
Contributor

@gotthardp gotthardp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest #168 instead.

@hyperfinitism
Copy link
Copy Markdown
Contributor Author

Thanks for the review and for pointing out the alternative approach.

I agree that the solution proposed in the other PR provides a clearer and more appropriate fix. In comparison, my change is more of a workaround and does not address the underlying issue as cleanly.

I will close this PR and follow the other one instead.

Thanks again for the feedback.

@hyperfinitism hyperfinitism deleted the fix/ivector-handling branch March 8, 2026 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants