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

`isRamSelected` is not updated when Ram is selected in `ChoosingRam::increaseValuesOfParticipants`

paprikrumplikas

Summary

State variable isRamSelected is not updated when Ram is selected ChoosingRam::increaseValuesOfParticipants.

Vulnerability Details

ChoosingRam::increaseValuesOfParticipants is supposed to select Ram for the event as soon as the value of an NFT reaches the maximum possible value (i.e. all boolean values in RamNFT::CharacteristicsOfRam is set to true). Even though this selection does happen, isRamSelected is not updated and not set to true.

This is demonstrated by the following test

function test_ramIsNotSelected() public {
vm.deal(player1, 1 ether);
vm.deal(player2, 1 ether);
​
vm.prank(player2);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
​
vm.startPrank(player1);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
choosingRam.increaseValuesOfParticipants(1, 0);
choosingRam.increaseValuesOfParticipants(1, 0);
choosingRam.increaseValuesOfParticipants(1, 0);
choosingRam.increaseValuesOfParticipants(1, 0);
choosingRam.increaseValuesOfParticipants(1, 0);
vm.stopPrank();
​
// NFT with tokenID = 0 reached maximum NFT value
assertEq(ramNFT.getCharacteristics(0).isJitaKrodhah, true);
assertEq(ramNFT.getCharacteristics(0).isAatmavan, true);
assertEq(ramNFT.getCharacteristics(0).isDhyutimaan, true);
assertEq(ramNFT.getCharacteristics(0).isSatyavaakyah, true);
assertEq(ramNFT.getCharacteristics(0).isVidvaan, true);
​
// isRamSelected has not been updated
assertEq(choosingRam.isRamSelected(), false);
}
​

Impact

  • initially selected Ram can be overwritten in ChoosingRam::increaseValuesOfParticipants if other NFT reaches the maximum possible value;

  • initally selected Ram can be overwritten by ChoosingRam::selectRamIfNotSelected;

  • functions Dussehra::killRavana and Dussehra::withdraw will always revert due to them being gated by the RamIsSelected modifier - unless organizer overwrites the initally selected Ram by calling ChoosingRam::selectRamIfNotSelected which would set isRamSelected = true; .

Tools Used

Manual review, Foundry.

Recommendations

Perform the following corrections in ChoosingRam:

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;
+ 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;
+ isRamSelected = true;
}
}
}
Updates

Lead Judging Commences

bube Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

`isRamSelected` is not set

Support

FAQs

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