First Flight #17: Dussehra

First Flight #17

Beginner FriendlyFoundryNFT

100 EXP

View results

Previous Next

Submission Details

Severity: high

Valid

`isRamSelected` variable is not updated in `ChoosingRam::increaseValuesOfParticipants` after Ram is selected, which prevents users from withdrawing fees.

feder

Summary

The increaseValuesOfParticipants function fails to update the isRamSelected variable after Ram is selected. This causes the Dussehra::killRavana function to revert, preventing users from withdrawing all the collected fees.

Vulnerability Details

It is not clear from the specification, but it seems like the idea behind the increaseValuesOfParticipants function is to allow two distinct tokens to "battle" against each other and provide the winner with a boost in experience. Once a token wins five battles, it can select Ram and access a reward.

After Ram is selected, two storage variables should be updated to reflect the new state of the system and allow other parts of the protocol to react accordingly.

The problem with the current implementation lies in the fact that the increaseValuesOfParticipants function fails to update the isRamSelected variable after Ram is selected. This failure causes the Dussehra::RamIsSelected modifier not to be updated properly. Consequently, when the killRavana function is called, it reverts due to the protocol being in an incorrect state.

function increaseValuesOfParticipants(uint256 tokenIdOfChallenger, uint256 tokenIdOfAnyPerticipent)

public

RamIsNotSelected

{

if (tokenIdOfChallenger > ramNFT.tokenCounter()) {

revert ChoosingRam__InvalidTokenIdOfChallenger();

}

if (tokenIdOfAnyPerticipent > ramNFT.tokenCounter()) {

revert ChoosingRam__InvalidTokenIdOfPerticipent();

}

if (ramNFT.getCharacteristics(tokenIdOfChallenger).ram != msg.sender) {

revert ChoosingRam__CallerIsNotChallenger();

}

if (block.timestamp > 1728691200) {

revert ChoosingRam__TimeToBeLikeRamFinish();

}

uint256 random =

uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao, msg.sender))) % 2;

if (random == 0) {

if (ramNFT.getCharacteristics(tokenIdOfChallenger).isJitaKrodhah == false) {

ramNFT.updateCharacteristics(tokenIdOfChallenger, true, false, false, false, false);

} else if (ramNFT.getCharacteristics(tokenIdOfChallenger).isDhyutimaan == false) {

ramNFT.updateCharacteristics(tokenIdOfChallenger, true, true, false, false, false);

} else if (ramNFT.getCharacteristics(tokenIdOfChallenger).isVidvaan == false) {

ramNFT.updateCharacteristics(tokenIdOfChallenger, true, true, true, false, false);

} else if (ramNFT.getCharacteristics(tokenIdOfChallenger).isAatmavan == false) {

ramNFT.updateCharacteristics(tokenIdOfChallenger, true, true, true, true, false);

} else if (ramNFT.getCharacteristics(tokenIdOfChallenger).isSatyavaakyah == false) {

ramNFT.updateCharacteristics(tokenIdOfChallenger, true, true, true, true, true);

selectedRam = ramNFT.getCharacteristics(tokenIdOfChallenger).ram;

// @audit missing isRamSelected = true;

}

} else {

if (ramNFT.getCharacteristics(tokenIdOfAnyPerticipent).isJitaKrodhah == false) {

ramNFT.updateCharacteristics(tokenIdOfAnyPerticipent, true, false, false, false, false);

} else if (ramNFT.getCharacteristics(tokenIdOfAnyPerticipent).isDhyutimaan == false) {

ramNFT.updateCharacteristics(tokenIdOfAnyPerticipent, true, true, false, false, false);

} else if (ramNFT.getCharacteristics(tokenIdOfAnyPerticipent).isVidvaan == false) {

ramNFT.updateCharacteristics(tokenIdOfAnyPerticipent, true, true, true, false, false);

} else if (ramNFT.getCharacteristics(tokenIdOfAnyPerticipent).isAatmavan == false) {

ramNFT.updateCharacteristics(tokenIdOfAnyPerticipent, true, true, true, true, false);

} else if (ramNFT.getCharacteristics(tokenIdOfAnyPerticipent).isSatyavaakyah == false) {

ramNFT.updateCharacteristics(tokenIdOfAnyPerticipent, true, true, true, true, true);

selectedRam = ramNFT.getCharacteristics(tokenIdOfAnyPerticipent).ram;

// @audit missing isRamSelected = true;

}

PoC

Add the following test to the test suite. It shows that, after Ram is selected and the killRavana function is called, the function reverts, preventing the organizer from receiving its reward.

function test_isRamSelectedNotSet() public participants {

// Player 1 selects Ram

vm.startPrank(player1);

choosingRam.increaseValuesOfParticipants(0, 0);

vm.stopPrank();

assertEq(choosingRam.selectedRam(), player1);

// Even though `selectedRam` variable is set correctly,

// the `isRamSelected` variable is still unset.

assertEq(choosingRam.isRamSelected(), false);

vm.warp(1728691200 + 1);

vm.expectRevert(bytes("Ram is not selected yet!"));

vm.startPrank(organiser);

dussehra.killRavana();

vm.stopPrank();

}

Impact

No fees can be withdrawn from the protocol, rendering it meaningless.

Tools Used

Manual review.

Recommended Mitigation

Update the isRamSelected storage variable after Ram is selected.