MyCut

First Flight #23
Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: medium
Valid

Potential Division by Zero in ```closePot``` Function

Summary

The closePot function in the Pot smart contract is vulnerable to division by zero if there is no claimant. If claimants.length is zero, the division operation for calculating claimantCut will revert the transaction, leading to a denial of service (DoS) and preventing the contract owner from closing the pot and distributing rewards.

Vulnerability Details

The issue is in this function:

https://github.com/Cyfrin/2024-08-MyCut/blob/main/src/Pot.sol#L49-L62

function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
i_token.transfer(msg.sender, managerCut);
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length; // i_players.length needs to be replaced with claimants.length
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}

NOTE: i_players.length needs to be replaced with claimants.length , however still there will be issues
The function calculates claimantCut by dividing the remaining rewards by claimants.length. If claimants.length is zero, this division by zero will cause the transaction to revert, blocking the execution of the function. The function should handle the case where there are no claimants to avoid division by zero. It should either skip the division operation or handle it in a way that ensures the transaction does not revert.

Proof of Concept (PoC):
Scenario:

  • The closePot function is called after the 90-day claim period has elapsed.

  • No claimants have been registered, resulting in claimants.length being zero.

Issue :

  • The calculation uint256 claimantCut = (remainingRewards - managerCut) / claimants.length will attempt to divide by zero.

  • This division by zero causes the transaction to revert, preventing the pot from being closed and the manager's cut from being claimed.

Impact

  • Denial of Service (DoS): The closePot function will fail if no claimants are present, locking the contract in a state where remaining rewards cannot be distributed, and the manager cannot claim their cut.

  • Contract Usability: Users and managers may experience issues if the contract cannot be properly closed due to the division by zero error.

Tools Used

Manual Review

Recommendations

To resolve this issue, add a check to ensure that claimants.length is greater than zero before performing the division. If no claimants are present, the function should either skip the claimant distribution or handle it in a way that avoids division by zero.

Updates

Lead Judging Commences

equious Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Incorrect handling of zero claimant edge case

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.