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

Users Can Challenge the NonExistant Last TokenId in `ChoosingRam::increaseValuesOfParticipants`

4rdiii

[L-1] Users Can Challenge the NonExistant Last TokenId in ChoosingRam::increaseValuesOfParticipants

Description:
The ChoosingRam::increaseValuesOfParticipants function contains checks to prevent users from using nonexistent token IDs. However, these checks use the > operator instead of >= when comparing token IDs against the current token counter (ramNFT.tokenCounter()). As a result, users can incorrectly challenge with the current last token ID, which does not exist yet.

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();
}
.
.
.
}

Impact:
Although this issue does not affect the outcome of ChoosingRam::increaseValuesOfParticipants—users challenging with the last token ID will still have the same chances of winning or losing—it contradicts the developer's intentions and could lead to confusion or misuse.

Proof of Concept:
To demonstrate this issue, add the following test to the existing test suite:

  1. get the lastTokenId from ramNFT

  2. check if the lastTokenId has a address attached to it

  3. Test the increaseValuesOfParticipants with lastTokenId

  4. Test the increaseValuesOfParticipants with lastTokenId + 1 to make sure it reverts with correct error

  5. check if the characteristics are updated for the challenge winner

function test_increaseValuesOfParticipantsCanBeUsedWithNonexistantTokenIds()
public
participants
{
uint256 lastTokenId = ramNFT.getNextTokenId();
RamNFT.CharacteristicsOfRam memory chars = ramNFT.getCharacteristics(
lastTokenId
);
assertEq(chars.ram, address(0));
vm.startPrank(player1);
choosingRam.increaseValuesOfParticipants(0, lastTokenId);
​
vm.expectRevert(
ChoosingRam.ChoosingRam__InvalidTokenIdOfPerticipent.selector
);
choosingRam.increaseValuesOfParticipants(0, lastTokenId + 1);
​
vm.stopPrank();
​
assertEq(ramNFT.getCharacteristics(lastTokenId).isJitaKrodhah, true);
}

Tools Used:
Manual Review

Recommended Mitigation:
Modify the token ID checks to use >= instead of > to correctly handle challenges involving the current last token ID, aligning with the developer's intentions.

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();
}
if (ramNFT.getCharacteristics(tokenIdOfChallenger).ram != msg.sender) {
revert ChoosingRam__CallerIsNotChallenger();
}
.
.
.
}
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.