FIX: FFI botan_cipher_update() for SIV and CCM #3971
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This happened for cipher modes that do not generate any output before they are finalized. The FFI adapter botan_cipher_update() failed to check the number of bytes generated by an update() call, and simply assumed that it would generate as many output bytes as it received input bytes.
randombit
approved these changes
Apr 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Dependencies
Description
As mentioned, the FFI function
botan_cipher_update()failed to properly handle cipher modes that produce output only after finalizing an input stream.With this patch, the function does not erroneously copy the input buffer into the output buffer anymore. As the copy happened from a zero-length byte vector, this might even be undefined behavior. Note that I didn't fix the potentially surprising fact that
botan_cipher_update()refuses to consume any input data if no equally sized output buffer is available.This adapts the test originally introduced in #3951 to be able to work with the existing implementation of
botan_cipher_update(). It acts as a regression test for the described behavior. Here, no performance improvements are applied tobotan_cipher_update(). Given that 3.4.0 is just around the corner, I thought it would be useful to create a minimal fix for the detected issue to be able to postpone the performance improvement until after the release.