Closed code423n4 closed 1 year ago
https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JBTiered721DelegateStore.sol#L688-L696
Solidity won't perform automatic checks when downcasting and it's possible for some fields to overflow while adding tiers.
JBTiered721DelegateStore.recordAddTiers()
_tiersToAdd
votingUnits
_storedTierOf
Similar behavior can occur for other fields for a tier.
Slient overflows will affect tier accouting and can cause unexpected behavior in the protocol.
Make use of a safe-cast library. E.g. OpenZeppelin's SafeCast.
Nice finding, disagree with severity (another tier can be added to fix it/no function/availability of the protocol impacted)
Picodes marked the issue as duplicate
Picodes marked the issue as satisfactory
Lines of code
https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JBTiered721DelegateStore.sol#L688-L696
Vulnerability details
Solidity won't perform automatic checks when downcasting and it's possible for some fields to overflow while adding tiers.
Proof of Concept
JBTiered721DelegateStore.recordAddTiers()
, one item for_tiersToAdd
containsvotingUnits
bigger than the size of uint16, e.g. 65536._storedTierOf
will overflow, e.g. if the input is 65536 the value will be 0.Similar behavior can occur for other fields for a tier.
https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JBTiered721DelegateStore.sol#L688-L696
Impact
Slient overflows will affect tier accouting and can cause unexpected behavior in the protocol.
Recommended Mitigation Steps
Make use of a safe-cast library. E.g. OpenZeppelin's SafeCast.