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

Incorrect integer comparisons and buffer overflows #105

Closed
gvanas opened this issue Oct 20, 2022 · 0 comments
Closed

Incorrect integer comparisons and buffer overflows #105

gvanas opened this issue Oct 20, 2022 · 0 comments

Comments

@gvanas
Copy link
Collaborator

gvanas commented Oct 20, 2022

In some parts of the code, the comparison between a caller-provided size and a small value is incorrectly written. For instance, in SpongeAbsorb() (see KeccakSponge.inc), the comparison is written as partialBlock + instance->byteIOIndex > rateInBytes, where partialBlock depends on the caller-provided size, rateInBytes is a (small) constant and instance->byteIOIndex is never greater than rateInBytes. If partialBlock is close to the limit, the problem is that partialBlock+instance->byteIOIndex can overflow, yielding a number smaller than rateInBytes and the comparison is flawed. Instead, it should be written as partialBlock > rateInBytes - instance->byteIOIndex, where the right hand side never underflows since instance->byteIOIndex is never greater than rateInBytes.

Additionally, there are a couple of incorrect castings, like (unsigned int)(dataByteLen - i) that can cause an overflow when size_t is 64-bit.

Consequently, a buffer overflow can occur. For instance, in SpongeAbsorb(), the flawed comparison with a large value for partialBlock causes an overflow in the subsequent call to SnP_AddBytes.

@gvanas gvanas closed this as completed in fdc6fef Oct 20, 2022
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

No branches or pull requests

1 participant