Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: low
Invalid

Checks Effects Interaction in `withdraw` function

Summary

The withdraw function in the Dussehra contract violates the checks-effects-interactions pattern, which can lead to reentrancy vulnerabilities. This pattern is a crucial best practice in Solidity development to prevent attackers from exploiting reentrancy issues, potentially leading to loss of funds.

Vulnerability Details

In the current implementation of the withdraw function, the contract makes an external call to transfer Ether before updating the state, which can be exploited in a reentrancy attack:

function withdraw() public RamIsSelected OnlyRam RavanKilled {
if (totalAmountGivenToRam == 0) {
revert Dussehra__AlreadyClaimedAmount();
}
uint256 amount = totalAmountGivenToRam;
(bool success, ) = msg.sender.call{value: amount}(""); // External call
require(success, "Failed to send money to Ram");
totalAmountGivenToRam = 0; // State update
}

Impact

  • Reentrancy Attack: An attacker can re-enter the withdraw function before the state variable totalAmountGivenToRam is set to zero, allowing them to drain the contract’s funds.

  • Loss of Funds: The contract’s Ether balance can be significantly depleted if exploited, causing financial damage to the contract and its stakeholders.

Tools Used

Manual review

Recommendations

Follow the Checks-Effects-Interactions Pattern: Ensure that the contract’s state is updated before making any external calls.

function withdraw() public RamIsSelected OnlyRam RavanKilled nonReentrant {
if (totalAmountGivenToRam == 0) {
revert Dussehra__AlreadyClaimedAmount();
}
uint256 amount = totalAmountGivenToRam;
totalAmountGivenToRam = 0; // State update before external call
(bool success, ) = msg.sender.call{value: amount}("");
require(success, "Failed to send money to Ram");
emit AmountWithdrawn(msg.sender, amount);
}
Updates

Lead Judging Commences

bube Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Invalid - reentrancy in withdraw

The `withdraw` function sends the given amount to Ram. If the attacker calls the `withdraw` function again before the state variable is changed, the function will revert because there are no more funds in the contract. This reentrancy has no impact for the protocol. It is recommended to follow the CEI pattern, but this is informational.

Support

FAQs

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