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

Wrong if check in `ChoosingRam::increaseValuesOfParticipants` for `tokenIdOfAnyPerticipent`

kostov

Summary

ChoosingRam::increaseValuesOfParticipants check for tokenIdOfAnyPerticipent allows the user to enter nonexisting tokenId for their opponent.

Vulnerability Details

Let's say, there are only 2 NFTs created for now. After each NFT creation, the tokeCounter increases its value by 1, therefore, after that the tokenCounter will equal 2.

The first NFT created will be with tokenId equal to 0
The second NFT created will be with tokenId equal to 1

So if a user decides to call ChoosingRam::increaseValuesOfParticipants with 2 as an argument for tokenIdOfAnyPerticipent, the check below will pass, although, such a NFT token does not exist.

if (tokenIdOfAnyPerticipent > ramNFT.tokenCounter()) {
revert ChoosingRam__InvalidTokenIdOfPerticipent();
}

To test paste the code below in Dussehra.t.sol

function test_increaseValuesOfParticipantsByPassTokenIdOfParticipantCheck() public participants {
ramNFT.mintRamNFT(player2);
uint256 cuurrentTokenId = ramNFT.getNextTokenId() - 1;
uint256 nextTokenId = ramNFT.tokenCounter();
​
//Getting the owner of nextTokenId will revert, because such does not exist
vm.expectRevert(abi.encodeWithSelector(ERC721NonexistentToken.selector, nextTokenId));
address ownerNonExistent = ramNFT.ownerOf(nextTokenId);
console.log("owner ", ownerNonExistent);
​
vm.startPrank(player2);
//Player enters battle with nonexistent tokenId
choosingRam.increaseValuesOfParticipants(cuurrentTokenId, nextTokenId);
bool isJita = ramNFT.getCharacteristics(nextTokenId).isJitaKrodhah;
bool isSat = ramNFT.getCharacteristics(nextTokenId).isSatyavaakyah;
assertEq(isJita, true);
vm.stopPrank();
​
console.log("passed to here");
ramNFT.mintRamNFT(player2);
address owner = ramNFT.ownerOf(nextTokenId);
//When someone becomes the owner of the nonExistentTokenId, its stats resets
bool isJitaKrodhah = ramNFT.getCharacteristics(nextTokenId).isJitaKrodhah;
assertEq(isJitaKrodhah, false);
assertEq(owner, player2);
}

Impact

The challenger user will be participating against no one, so he can keep calling increaseValuesOfParticipants with either him becoming the Ram or making address 0 the Ram. If the 0 address, becomes the Ram then the user can call Dussehra::killRavana and send the reward to the 0 address.

Tools Used

Manual review

Unit test

Recommendations

Change the if check to:

+ if (tokenIdOfAnyPerticipent >= ramNFT.tokenCounter()) {
revert ChoosingRam__InvalidTokenIdOfPerticipent();
}
Updates

Lead Judging Commences

bube Lead Judge about 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.