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

Reentrancy attack in `Dussehra::withdraw`

kostov

Summary

Reentrancy attack in Dussehra::withdraw allows the Rama to withdraw a bigger amount from the contract.

Vulnerability Details

CEI is not being followed in Dussehra::withdraw which allows the Ram to keep re-entering the function, while the contract has ETH.

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

Impact

This allows the Rama to withdraw all the remaining money from the contract, however, before the Rama would be able to withdraw their reward, they have to call killRavana which transfers half the contract's amount to the organizer. So if users don't keep calling enterPeopleWhoLikeRam even after the Rama has been killed, the Rama won't be able to get an extra amount.

Tools

Manual review

Recommendations

Update totalAmountGivenToRam before sending the ETH to the Rama

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

Lead Judging Commences

bube Lead Judge over 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.

Give us feedback!