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

Wrong check in `ChoosingRam::increaseValuesOfParticipants` allows users to become Ram by challenging an unexisted participant

NightHawK

Summary

Wrong checks on Line 37 and Line 40 of the ChoosingRam contract allows the users to challenge an unexisted participant. This increases their chance to become Ram and get the reward.

Vulnerability Details

The ChoosingRam::increaseValuesOfParticipants function is expected to revert when an unexisted token id is challenged. For this purpose the RamNFT contract counts the number of minted NFT tokens and provides the function RamNFT::tokenCounter. The returned value of this function is not used correctly in the equation in Line 37 of the ChoosingRam. The equation must be tokenIdOfChallenger >= ramNFT.tokenCounter() instead of tokenIdOfChallenger > ramNFT.tokenCounter(). Also for Line 40 the equation must be tokenIdOfAnyPerticipent >= ramNFT.tokenCounter() and not tokenIdOfAnyPerticipent > ramNFT.tokenCounter().

Proof of Concept

The following test shows that the ChoosingRam::increaseValuesOfParticipants is not reverting as expected.

function test_usersCanChallengeUnexistedToken() public {
Dussehra dussehra;
RamNFT ramNFT;
ChoosingRam choosingRam;
address organiser = makeAddr("organiser");
address player1 = makeAddr("player1");
​
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.warp(2);
// there is only one participant with only one RamNFT token
// but the increaseValuesOfParticipants function will not revert
// if token ids 0 and 1 are used
choosingRam.increaseValuesOfParticipants(0, 1);
choosingRam.increaseValuesOfParticipants(0, 1);
choosingRam.increaseValuesOfParticipants(0, 1);
choosingRam.increaseValuesOfParticipants(0, 1);
choosingRam.increaseValuesOfParticipants(0, 1);
vm.stopPrank();
​
// and finally the only player will be Ram
// without winning any challenge with another player
// actually, even there is no another player who had
// entered the contest
assertEq(choosingRam.selectedRam(), player1);
}

Impact

The users might get the information from the RamNFT::tokenCounter and always challenge an unexisted token which will increase their chance to become Ram and get the reward.

Tools Used

Manual review

Recommendations

Fix the sign of the equations in Line 37 and Line 40 of the ChoosingRam. See the code below.

function increaseValuesOfParticipants(
uint256 tokenIdOfChallenger,
uint256 tokenIdOfAnyPerticipent
) public RamIsNotSelected {
- if (tokenIdOfChallenger > ramNFT.tokenCounter()) {
+ if (tokenIdOfChallenger >= ramNFT.tokenCounter()) {
revert ChoosingRam__InvalidTokenIdOfChallenger();
}
- if (tokenIdOfAnyPerticipent > ramNFT.tokenCounter()) {
+ if (tokenIdOfAnyPerticipent >= ramNFT.tokenCounter()) {
revert ChoosingRam__InvalidTokenIdOfPerticipent();
}
}
Updates

Lead Judging Commences

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

The token counter check is incorrect

Support

FAQs

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