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

Modify SymmetricCipher overlaps buffer checks #295

Merged

Conversation

jasonkatonica
Copy link
Member

The doFinal and update methods in the SymmetricCipher class have logic to detect if a input buffer and output buffer are overlapping in any sort of way. If they are then a copy of the input is made to a separate location to ensure a safe operation can then take place on the data.

There is a logic problem where the calculation of overlap was incorrect and used a = instead of a + to calculate if the input and output overlapped. The comment above this code block was correct and remains unmodified.

The test BaseTestAESGCMCopySafe was modified to test this condition which it was attempting to do in the past however it was not operating upon input and output buffers that had the same memory address. The test was modified to exercise various overlapping conditions for AES GCM and also AES CBC accordingly. The test was then renamed to BaseTestAESCopySafe since we are now testing both GCM and CBC modes.

Fixes #292

Signed-off-by: Jason Katonica katonica@us.ibm.com

Copy link
Member

@KostasTsiounis KostasTsiounis left a comment

Choose a reason for hiding this comment

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

LGTM

The `doFinal` and `update` methods in the SymmetricCipher class have
logic to detect if a input buffer and output buffer are overlapping
in any sort of way. If they are then a copy of the input is made to a
separate location to ensure a safe operation can then take place on the
data.

There is a logic problem where the calculation of overlap was incorrect
and used a `=` instead of a `+` to calculate if the input and output
overlapped. The comment above this code block was correct and remains
unmodified.

The test `BaseTestAESGCMCopySafe` was modified to test this condition
which it was attempting to do in the past however it was not operating
upon input and output buffers that had the same memory address. The
test was modified to exercise various overlapping conditions for AES GCM
and also AES CBC accordingly. The test was then renamed to
`BaseTestAESCopySafe` since we are now testing both GCM and CBC modes.

Fixes IBM#292

Signed-off-by: Jason Katonica <katonica@us.ibm.com>
@jasonkatonica jasonkatonica force-pushed the katonica/issue292/fixoverlaplogic branch from 46a71f1 to dee50b3 Compare October 28, 2024 18:33
Copy link
Collaborator

@taoliult taoliult left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@JinhangZhang JinhangZhang left a comment

Choose a reason for hiding this comment

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

LGTM

@jasonkatonica
Copy link
Member Author

x86_64_linux,ppc64le_linux,s390x_linux,x86_64_windows

@jasonkatonica jasonkatonica merged commit d8041a8 into IBM:main Oct 30, 2024
2 checks passed
@jasonkatonica jasonkatonica deleted the katonica/issue292/fixoverlaplogic branch October 30, 2024 12:18
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.

Fix unary oparator in the SymmetricCipher
5 participants