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

C.E.I not followed in `Dussehra::withdraw`. Re-entrancy is possible.

modey

Summary

A Ram calling the Dussehra::withdraw function, can reenter the function maliciously and withdraw more than once because Check-Effects-Interactions pattern was not followed.

Vulnerability Details

The Dussehra::withdraw function allows a selected Ram to withdraw uint256 amount = totalAmountGivenToRam. But in the implementation of this withdrawal, totalAmountGivenToRam is reset to zero just after the withdrawal is done. Which goes against Check-Effects-Interactions pattern thereby opening a surface for reentrancy attack, and enables the caller to possibly reenter the function and get paid again because the state change was not done before payment functionality.

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");
​
//reentrancy vulnerability. State change being made after external interaction
@> totalAmountGivenToRam = 0;
}
​

Impact

The Ram can steal funds from contract.

Tools Used

Manual

Recommendations

Follow C-E-I Pattern and ensure state changes are made before external interactions. Please consider how this is displayed below:

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.