fix: fix uint64->uint32 underflow issues resulting from using cosmossdk.io/math #293
Conversation
WalkthroughThe modification in the codebase focuses on enhancing security and reliability by replacing the use of Changes
Assessment against linked issues
Recent review detailsConfiguration used: CodeRabbit UI Files ignored due to path filters (2)
Files selected for processing (6)
Additional comments not posted (25)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
odeke-em
force-pushed
the
fix-x-auction-client-underflow-issues
branch
from
a882442
to
af08855
Compare
odeke-em
changed the title
odeke-em
force-pushed
the
fix-x-auction-client-underflow-issues
branch
3 times, most recently
from
3425674
to
9f66b41
Compare
|
Thank you for the PR. Is this something that has been fixed in strconv in the intervening time since you opened it? If you find time would you mind merging main? |
…dk.io/math
This fixes potential security issues resulting from extraneous
parsing that used cosmossdk.math.ParseUint which uses math/big.Int
which is a big integer package and can allow uint64 in places
where uint32 is used and there were underflow checks.
The fix for this change was to simply use
strconv.ParseUint(s, 10, BIT_SIZE)
where BIT_SIZE is any of 32 or 64 bits for uint32 and uint64 respectively,
and by extracting the shared code to an internal package.
Fixes PeggyJV#292
odeke-em
force-pushed
the
fix-x-auction-client-underflow-issues
branch
from
9f66b41
to
068f594
Compare
|
Hey @cbrit, I've rebased from the latest on main. Please take a look again.
So the problems weren't in strconv but rather in the usage of it in these libraries, without careful consideration. |
This fixes potential security issues resulting from extraneous
parsing that used cosmossdk.math.ParseUint which uses math/big.Int
which is a big integer package and can allow uint64 in places
where uint32 is used and there were underflow checks.
The fix for this change was to simply use
where BIT_SIZE is any of 32 or 64 bits for uint32 and uint64 respectively,
and by extracting the shared code to an internal package.
Fixes #292
Summary by CodeRabbit
New Features
Bug Fixes
Refactor