First Flight #17: Dussehra

Summary

The Dussehra::withdraw() function doesn't have any mechanism to prevent a reentrancy attack and doesn't follow the Check-effects-interactions pattern

Vulnerability Details

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

In the provided Dussehra contract is potentially vulnerable to reentrancy attacks. This is because it first sends Ether to msg.sender and then updates the state of the contract.a malicious contract could re-enter the refund function before the state is updated.

Impact

If exploited, this vulnerability could allow a malicious contract to drain Ether from the Dussehra contract, leading to loss of funds for the contract and its users.

Tools Used

Manual Review

Recommendations

To mitigate the reentrancy vulnerability, you should follow the Checks-Effects-Interactions pattern. This pattern suggests that you should make any state changes before calling external contracts or sending Ether.

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");
}