Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix mbedtls_cipher_finish invocations #122

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

justinkb
Copy link

@justinkb justinkb commented Dec 4, 2023

those mbedtls_cipher_finish calls never did anything like this, since we were passing a null pointer for the third argument and the old included mbedtls just returns an error code (which we completely ignore) if you do that, refer to https://github.com/SciresM/hactool/blob/master/mbedtls/library/cipher.c#L684 for that.

I ran into this when I was building hactool against mbedtls 3.5.1 instead of the repo included one, it segfaults with that version (on dereferencing that third argument pointer), since they removed null pointer checks along the way to go with their stricter requirements on what you can pass as arguments.

you can argue this doesn't really meaningfully change or fix the operation of hactool, since all that crypto stuff is just transient anyway and any potential bad state issue evaporates once the program stops execution, but still I thought it was worth submitting this change.

&out_len reuse as output parameter for the mbedtls_cipher_finish is fine, since we never need that actual value from mbedtls_cipher_update invocation(s) anyway except now with this change to cleanly do the mbedtls_cipher_finish with the correct 2nd argument pointer

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.

1 participant