howlbot-integration[bot] commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-07-munchables/blob/94cf468aaabf526b7a8319f7eba34014ccebe7b9/src/managers/LandManager.sol#L140

Vulnerability details

Impact

Users can stake more than the intended maximum of 10 NFTs potentially gaining additional rewards or benefits that were not intended by the system's design. Additionally, It may lead to unexpected behavior if other parts of the system assume a maximum of 10 staked NFTs per Account.

Proof of Concept

The current implementation of the stakeMunchable function includes a check to enforce a maximum limit of 10 staked NFTs per account. The condition used to enforce this limit is:

if (munchablesStaked[mainAccount].length > 10) revert TooManyStakedMunchiesError();

However, this condition only reverts when the number of staked NFTs exceeds 10, meaning an account can stake exactly 11 NFTs without triggering the error. This bypass occurs because the condition checks for > (greater than) rather than >= (greater than or equal to).

If for example an account with 10 staked NFTs calls the stake function, it won't revert and will proceed to stake an additional NFT

Tools Used

Manual Review

Recommended Mitigation Steps

To correctly enforce the intended limit of 10 NFTs per account, the condition should be updated to:

if (munchablesStaked[mainAccount].length >= 10) revert TooManyStakedMunchiesError();

This change ensures that any attempt to stake more than 10 NFTs will correctly revert adhering to the system's rules and preventing the potential exploitation of the staking limit.

Assessed type

Invalid Validation

c4-judge commented 1 month ago

alex-ppg changed the severity to QA (Quality Assurance)