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

Missing input validation in `ChoosingRam::increaseValuesOfParticipants()` enables users to provide the same argument for both parameters

Summary

The ChoosingRam::increaseValuesOfParticipants() function is intended to increase the attributes of the user's RamNFT. When a RamNFT is minted, it stores the address of the minter (RamNFT::CharacteristicsOfRam.ram) and defaults 5 characteristics to false:

Characteristics[newTokenId] = CharacteristicsOfRam({
ram: to,
isJitaKrodhah: false,
isDhyutimaan: false,
isVidvaan: false,
isAatmavan: false,
isSatyavaakyah: false
});

When all of these characteristics are set to true, RamNFT::CharacteristicsOfRam.ram becomes the ChoosingRam::selectedRam. The ChoosingRam::selectedRam has access to the Dussehra::withdraw() function. Dussehra::withdraw() allows the ChoosingRam::selectedRam to have a share of the funds derived from each entry into the protocol (Dussehra::totalAmountGivenToRam)

The ChoosingRam::increaseValuesOfParticipants() function requires two uint256 arguments

function increaseValuesOfParticipants(uint256 tokenIdOfChallenger, uint256 tokenIdOfAnyPerticipent)

The ChoosingRam::increaseValuesOfParticipants() function will change one RamNFT::CharacteristicsOfRam to true per function call. The function will either manipulate msg.sender's RamNFT, or it will manipulate the RamNFT of another participant (the second argument). It should take at least 5 function calls to flip all characteristics to true:

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(tokenIdOfAnyPerticipent).isJitaKrodhah == false){
ramNFT.updateCharacteristics(tokenIdOfAnyPerticipent, true, false, false, false, false);
} else if (ramNFT.getCharacteristics(tokenIdOfAnyPerticipent).isDhyutimaan == false){
ramNFT.updateCharacteristics(tokenIdOfAnyPerticipent, true, true, false, false, false);
} else if (ramNFT.getCharacteristics(tokenIdOfAnyPerticipent).isVidvaan == false){
ramNFT.updateCharacteristics(tokenIdOfAnyPerticipent, true, true, true, false, false);
} else if (ramNFT.getCharacteristics(tokenIdOfAnyPerticipent).isAatmavan == false){
ramNFT.updateCharacteristics(tokenIdOfAnyPerticipent, true, true, true, true, false);
} else if (ramNFT.getCharacteristics(tokenIdOfAnyPerticipent).isSatyavaakyah == false){
ramNFT.updateCharacteristics(tokenIdOfAnyPerticipent, true, true, true, true, true);
selectedRam = ramNFT.getCharacteristics(tokenIdOfAnyPerticipent).ram;
}
}

The spirit of this function is to utilize "randomness" to flip characteristics to true. The intention is to require users to call ChoosingRam::increaseValuesOfParticipants() multiple times (probably much more than the minimum of 5) in order to become ChoosingRam::selectedRam. We can make this assumption based on this check:

if (ramNFT.getCharacteristics(tokenIdOfChallenger).ram != msg.sender) {
revert ChoosingRam__CallerIsNotChallenger();
}

This check ensures that msg.sender is the owner of the RamNFT with tokenIdOfChallenger id. So we can assume the idea is for the user to input their tokenId and another tokenId. However, because there is no validation comparing the two arguments (uint256 tokenIdOfChallenger and uint256 tokenIdOfAnyPerticipent), the system can easily be gamed.

Vulnerability Details

This foundry test shows how easy it is to game the system:

function test_SelectYourself() public participants {
vm.startPrank(player1);
choosingRam.increaseValuesOfParticipants(0, 0);
assertEq(ramNFT.getCharacteristics(0).isJitaKrodhah, true);
choosingRam.increaseValuesOfParticipants(0, 0);
assertEq(ramNFT.getCharacteristics(0).isDhyutimaan, true);
choosingRam.increaseValuesOfParticipants(0, 0);
assertEq(ramNFT.getCharacteristics(0).isVidvaan, true);
choosingRam.increaseValuesOfParticipants(0, 0);
assertEq(ramNFT.getCharacteristics(0).isAatmavan, true);
choosingRam.increaseValuesOfParticipants(0, 0);
assertEq(ramNFT.getCharacteristics(0).isSatyavaakyah, true);
assertEq(choosingRam.selectedRam(),player1);
vm.stopPrank();
}

The user inputs their tokenId twice. They successfully flip each characteristic to true in each function call requiring a total of only 5 function calls. The user becomes ChoosingRam::selectedRam.

Impact

This is a high risk vulnerability that allows a user to game the system.

Abusing a lack of input validation, there is nothing stopping a user from using their own tokenId as both arguments for ChoosingRam::increaseValuesOfParticipants(). This allows any user to quickly become ChoosingRam::selectedRam in 5 function calls. As speculated earlier, it is unlikely that this is intended behavior. The intentions are to create a system dictated by 'randomness' and luck. The lack of input comparison bypasses the need for luck.

The point of this gamified NFT protocol is to gain ChoosingRam::selectedRam status. With this status, the user gains access to the Dussehra::withdraw() function, giving the user a claim to 50% of the RamNFT mint.

The ChoosingRam::increaseValuesOfParticipants() is very easy to manipulate, and it entitles the manipulator to 50% of the funds.

Tools Used

  • Manual Review

  • Foundry

Recommendations

The easiest form of mitigation would be to include input validation to ensure both uint256 arguments are different

if (tokenIdOfChallenger == tokenIdOfAnyPerticipent) {
revert ChoosingRam__Cheater();
}
  • However, the system can still be gamed: enter the raffle twice with different addresses, and use your second RamNFT as the second parameter.

A better solution is to have the function randomly choose a second participant. This solution will require more gas, but it won't allow msg.sender to freely choose the second input parameter.

+ function increaseValuesOfParticipants(uint256 tokenIdOfChallenger)
- function increaseValuesOfParticipants(uint256 tokenIdOfChallenger, uint256 tokenIdOfAnyPerticipent)
public
RamIsNotSelected
{
+ // random user selected based on `RamNFT::getNextTokenId` using ChainlinkVRF
//...rest of function...
}
Updates

Lead Judging Commences

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

Challenge themselves

Support

FAQs

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