Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: high
Valid

Non-existant token id can be parameter in `ChoosingRam::increaseValuesOfParticipants` function which can lead to increasing value of non-existant token.

Summary

The ChoosingRam::increaseValuesOfParticipants function allows for the possibility of increasing the value of non-existent tokens due to improper validation checks on token IDs. This can lead to unintended behavior and potential manipulation of the game mechanics, as non-existent tokens should not have their values increased.

Vulnerability Details

function increaseValuesOfParticipants(uint256 tokenIdOfChallenger, uint256 tokenIdOfAnyPerticipent)
public
RamIsNotSelected
{
if (tokenIdOfChallenger > ramNFT.tokenCounter()) {
revert ChoosingRam__InvalidTokenIdOfChallenger();
}
// @audit - wrong operator
@> if (tokenIdOfAnyPerticipent > ramNFT.tokenCounter()) {
revert ChoosingRam__InvalidTokenIdOfPerticipent();
}

ChoosingRam::increaseValuesOfParticipants function is used to increase value of Ram NFT. Challenger is calling function and plays against participant by providing his token id and token id of participant.

In this function there is check that token id of participant must be less than or equal to token counter. This is not correct, because RamNFT::tokenCounter returns number of tokens that exist and minting in RamNFT contract starts from token id 0. So if there are 2 tokens, token id 0 and token id 1 exist, but token id 2 does not exist yet. If token id 2 is used as parameter, function will not revert which is incorrect.

  1. Player 1 and player 2 mint their Ram NFT, token id 0 and 1. Token id 2 does not exist yet.

  2. Player 1 with token id 0 plays against non-existant token id 2.

  3. Value of token id 0 or token id 2 is increased based on random value.

  4. Assert that value of token id 0 or token id 2 is increased to Jita Krodhah.

PoC

Place the following test into Dussehra.t.sol.

function test_challengerCanInputInvalidTokenIdForParticipant() public participants {
address owner = (ramNFT.getCharacteristics(2)).ram;
// Assert that nobody owns token id 2
assertTrue(owner == address(0));
bool challengerValue = ramNFT.getCharacteristics(0).isJitaKrodhah;
bool participantValue = ramNFT.getCharacteristics(2).isJitaKrodhah;
assertTrue(challengerValue == false && participantValue == false);
vm.startPrank(player1);
choosingRam.increaseValuesOfParticipants(0, 2);
challengerValue = ramNFT.getCharacteristics(0).isJitaKrodhah;
participantValue = ramNFT.getCharacteristics(2).isJitaKrodhah;
assertTrue(challengerValue == true || participantValue == true);
}

Impact

Token that currently does not exist can increase its value. One player could play multiple times against this non-existant token, because its win-win situation for him. If that non-existant token accrues bigger value, player just needs to mint that token.

Another situation is that if non-existant increase value to become selected Ram, it will be possible to kill Ravana by calling Dussehra::killRavana function even if no one mints that token.

Tools Used

Manual review

Recommendations

Change operator in ChoosingRam::increaseValuesOfParticipants function to >=.

function increaseValuesOfParticipants(uint256 tokenIdOfChallenger, uint256 tokenIdOfAnyPerticipent)
public
RamIsNotSelected
{
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
about 1 year ago
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.