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

`Dussehra::withdraw` function doesn't follow CEI pattern

codexbugmenot

Summary

The Dussehra::withdraw function should follow the CEI(Checks-Effects-Interactions) pattern, here it breaks the pattern by making a state change after the transfer to msg.sender is done which can allow the msg.sender to reenter the function.

Vulnerability Details

The Dussehra::withdraw function doesn't follow the CEI pattern,
It is violated by the line

totalAmountGivenToRam = 0;

which is making a state change after the transfer is done allowing a msg.sender to reenter the function and call Dussehra::withdraw again

​
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;
}

Impact

The Reentrancy lies dormant in the Dussehra::withdraw function with no real impact for now since after the initial withdraw the contract is left with no balance but if there were any changes in the future then there will always be a possibility for the msg.sender to reenter the function to steal funds

Tools Used

  1. Manual Review

  2. Foundry

Recommendations

The Dussehra::withdraw function should follow the CEI pattern i.e make the state change before the transfer is done and use OpenZeppelins non-reentrant modifier to prevent reentrancy.

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.