First Flight #17: Dussehra
First Flight #17
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

Reentrancy in Dussehra.sol::withdraw()

kiinzu

Summary

The withdraw() function doesn't implement CEI (Check-Effect-Interaction) pattern, causing the function vulnerable to Reentrancy Attack.

Vulnerability Details

Once Ram is selected, the function killRavana() is now accessible, and once the Ravana dies, the chosen Ram can then proceed to take the money presented to him in the totalAmountGivenToRam.

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

The withdraw() implements CIE, which could let an attacker call it multiple times while inside the receive() or fallback() function inside the attacking contract, making it always stop on every successful transfer then repeat the function call and won't change the totalAmountGivenToRam variable.

Impact

The money that the Dussehra.sol contract holds could be drained by the attack.

Tools Used

Manual Analysis

Recommendations

Slightly modify the pattern to use a best practice of CEI, like the code below.

function withdraw() public RamIsSelected OnlyRam RavanKilled {
if (totalAmountGivenToRam == 0) {
revert Dussehra__AlreadyClaimedAmount();
}
uint256 amount = totalAmountGivenToRam;
- (bool success, ) = msg.sender.call{value: amount}("");
- require(success, "Failed to send money to Ram");
- totalAmountGivenToRam = 0;
+ totalAmountGivenToRam = 0;
+ (bool success, ) = msg.sender.call{value: amount}("");
+ require(success, "Failed to send money to Ram");
}
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.