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

Unnecessarily Restrictive Time Constraints for `ChoosingRam::selectRamIfNotSelected` And `Dussehra::killRavana` Can Result in Funds Permanently Being Locked in the Contract

vile

Summary

A period of time exists where block stuffing can be abused to prevent Dussehra::killRavana from being executed within its time constraints, resulting in timestamp checks ALWAYS failing. This guarantees calls to Dussehra::killRavana and Dussehra::withdraw revert, preventing intended users from receiving ETH and permanently locking funds in the contract.

Vulnerability Details

A specific series of events must occur for the Dussehra::killRavana and Dussehra::withdraw functions to be callable and send their designated users ETH.

The series of events is as follows (as currently implemented; all timestamps are in UTC):

  • selectRamIfNotSelected MUST be called before or at Sun Oct 13 2024 00:00:00

    • This sets isRamSelected to true (increaseValuesOfParticipants currently DOES NOT set this value)

    • isRamSelected is required to pass the RamIsSelected modifier on killRavana and withdraw

  • killRavana MUST be called before or at Sun Oct 13 2024 00:01:09

    • This sets IsRavanKilled to true

  • withdraw can now be called, at any time, because isRamSelected and IsRavanKilled are set to true.

function selectRamIfNotSelected() public RamIsNotSelected OnlyOrganiser {
// Fri Oct 11 2024 23:57:49 GMT+0000
if (block.timestamp < 1728691200) {
revert ChoosingRam__TimeToBeLikeRamIsNotFinish();
}
// Sun Oct 13 2024 00:01:09 GMT+0000
if (block.timestamp > 1728777600) {
revert ChoosingRam__EventIsFinished();
}
// ...
isRamSelected = true;
}
function killRavana() public RamIsSelected {
// Fri Oct 11 2024 23:57:49 GMT+0000
if (block.timestamp < 1728691069) {
revert Dussehra__MahuratIsNotStart();
}
// Sun Oct 13 2024 00:01:09 GMT+0000
if (block.timestamp > 1728777669) {
revert Dussehra__MahuratIsFinished();
}
IsRavanKilled = true;
// ...
}

However, if the organiser calls selectRamIfNotSelected at the latest possible time (Sun Oct 13 2024 00:00:00), there exists a one minute and ten second period where killRavana MUST be called, otherwise killRavana will ALWAYS revert due to timestamp checks. Furthermore this prevents IsRavanKilled from being set to true, and prevents ALL withdraw calls from succeeding (no matter the time).

A malicious user can abuse this by stuffing blocks for the one minute and ten seconds period, preventing the ANY killRavana calls (since it is a public function) being included in a block and executed.

Add the following test case to Dussehra.t.sol:

Test Case
function test_killRavanaCanBeBlockStuffed() public participants {
// block.timestamp = 0, meaning token id 0 will always be chosen for `random`
vm.warp(0);
vm.startPrank(player1);
choosingRam.increaseValuesOfParticipants(0, 1);
choosingRam.increaseValuesOfParticipants(0, 1);
choosingRam.increaseValuesOfParticipants(0, 1);
choosingRam.increaseValuesOfParticipants(0, 1);
choosingRam.increaseValuesOfParticipants(0, 1);
vm.stopPrank();
​
// `player1` has obtained all characteristics for their NFT, however `isRamSelected` was never set to `true`
assertEq(choosingRam.selectedRam(), address(player1));
assertFalse(choosingRam.isRamSelected());
​
// since `isRamSelected` is still false, `Dussehra::killRavana` cannot be called
vm.prank(player1);
vm.expectRevert("Ram is not selected yet!");
dussehra.killRavana();
​
// now, the `organiser` waits until the latest possbile moment to call `selectRamIfNotSelected`
vm.warp(1728777600);
vm.prank(organiser);
choosingRam.selectRamIfNotSelected();
​
// now, `isRamSelected` has been set to `true` by this call
assertTrue(choosingRam.isRamSelected());
// at the 1728777600 timestamp, `player2` is deterministically chosen
assertEq(choosingRam.selectedRam(), address(player2));
​
// a malicious user then stuffs blocks for one minute and ten seconds
vm.warp(1728777669 + 1);
​
// `killRavana` now cannot be called; this can be called by anyone, but we prank for clarity
vm.prank(player2);
vm.expectRevert(Dussehra.Dussehra__MahuratIsFinished.selector);
dussehra.killRavana();
​
// `IsRavanKilled` cannot be set to `true`
assertFalse(dussehra.IsRavanKilled());
​
// this also prevents `withdraw` from being called, even if they are the selected ram
assertEq(choosingRam.selectedRam(), address(player2));
​
vm.prank(player2);
vm.expectRevert("Ravan is not killed yet!");
dussehra.withdraw();
​
// ether is never withdrawn, and is now permanently stuck
assertEq(address(organiser).balance, 0 ether);
assertEq(address(player2).balance, 0 ether);
assertEq(address(dussehra).balance, 2 ether); // 1 ether (entranceFee) * 2
}

Then, run the test:

forge test --mt test_killRavanaCanBeBlockStuffed -vvvvv

Impact

After the block stuffing has occurred, funds intended for BOTH the organiser AND selected selectedRam are permanently locked in the contract.

Tools Used

Manual Review

Recommendations

Greater-than timestamp checks in both selectRamIfNotSelected and killRavana should be removed to allow them to be called at any time in the future, preventing funds from becoming permanently locked.

function selectRamIfNotSelected() public RamIsNotSelected OnlyOrganiser {
if (block.timestamp < 1728691200) {
revert ChoosingRam__TimeToBeLikeRamIsNotFinish();
}
- if (block.timestamp > 1728777600) {
- revert ChoosingRam__EventIsFinished();
- }
// ...
isRamSelected = true;
}
}
function killRavana() public RamIsSelected {
if (block.timestamp < 1728691069) {
revert Dussehra__MahuratIsNotStart();
}
- if (block.timestamp > 1728777669) {
- revert Dussehra__MahuratIsFinished();
- }
IsRavanKilled = true;
// ...
}
Updates

Lead Judging Commences

bube Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
vile Submitter
about 1 year ago
bube Lead Judge
about 1 year ago
vile Submitter
about 1 year ago
bube Lead Judge
about 1 year ago
vile Submitter
about 1 year ago
bube Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.