Summary
According to the documentation the Ram must be set to selected when someone gets all of the characteristics. The characteristics are increased by the ChoosingRam::increaseValuesOfParticipants function. However, this function does not set the flag ChoosingRam::isRamSelected to true when someone gets all of the characteristics.
Vulnerability Details
The ChoosingRam::increaseValuesOfParticipants function is supposed to revert when Ram is selected. The revert is based on the flag ChoosingRam::isRamSelected but this flag is never set by the function. So, even after someone gets all the characterics the function will not revert. This will allow another user to be selected as Ram and so on.
Proof of Concept
The following code demonstrates the problem.
function test_ramIsNotSetToSelected() public {
Dussehra dussehra;
RamNFT ramNFT;
ChoosingRam choosingRam;
address organiser = makeAddr("organiser");
address player1 = makeAddr("player1");
address player2 = makeAddr("player2");
vm.startPrank(organiser);
ramNFT = new RamNFT();
choosingRam = new ChoosingRam(address(ramNFT));
dussehra = new Dussehra(1 ether, address(choosingRam), address(ramNFT));
ramNFT.setChoosingRamContract(address(choosingRam));
vm.stopPrank();
vm.startPrank(player1);
vm.deal(player1, 1 ether);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
vm.stopPrank();
vm.startPrank(player2);
vm.deal(player2, 1 ether);
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();
assertEq(choosingRam.selectedRam(), player1);
assertEq(choosingRam.isRamSelected(), false);
}
Impact
The missing set of the flag will allow the users to challenge other users and change the selected Ram multiple times which breaks the expected logic. The Ram who will take the reward will not be the correct one.
Tools Used
Manual review
Recommendations
Set the flag ChoosingRam::isRamSelected to true when the Ram is selected. See the code below.
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;
}
}
}
