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

Function `Dussehra::withdraw` open to reentracy attack

appet

Description:

Function Dussehra::withdraw does not follow CEI rules - the value of totalAmountGivenToRam variable is changed after executing msg.sender.call{value: amount}(""); line

Impact:

Although there is a possibility for reentrancy attack, such could be initiated only by OnlyRam (i.e. by the selected user for Ram) which is indeed the winner of the event and should take the prize.

Proof of Concept:

This is a classic example of the reentrancy type of attack. Please refer for more information here :
https://solidity-by-example.org/hacks/re-entrancy/

Recommended Mitigation:

Move the totalAmountGivenToRam = 0; line before the .call functionality.

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

Tools Used

Manual review

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.