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

`ChoosingRam:: increaseValuesOfParticipants` is not implemented correctly which allows selecting Ram multiple times, overrides the prev one

abhishekthakur

Summary

Allows overriding selected Ram which is not the intended behaviour. Moreover allows user to update his nft to Ram with calculated steps due to lack of id duplicate check.

Vulnerability Details

increaseValuesOfParticipants allows choosing Ram multiple time as per current logic. which is not the intended behaviour of the protocol. Contract should only allow choosing Ram once and should revert afterwards.
Also, function take id as input, challanger and id of any participants which is not checked for duplicate values. A user can put his own id in both places and become Ram by calling it 5 times.
Current implementation will make latest caller as Ram, who has overridden the Prev one.

Impact

It makes this function useless, as Ram still will be selected by organiser. This will make users unhappy who spent eth for nft minting, as it reduces there odds of becoming Ram to large extent.

Tools Used

Manual Review, Foundry

Recommendations

update the boolean flag as soon as Ram is selected.
add checks to avoid same id as input.
Here is the updated version -

​
+ error ChoosingRam__ChallangerAndParticipentNFTMustBeUnique();
​
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();
}
+ if(tokenIdOfChallenger == tokenIdOfAnyPerticipent) {
+ revert ChoosingRam__ChallangerAndParticipentNFTMustBeUnique();
+ }
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);
+ isRamSelected = 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);
+ isRamSelected = true;
selectedRam = ramNFT.getCharacteristics(tokenIdOfAnyPerticipent).ram;
}
}
}
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:

Challenge themselves

`isRamSelected` is not set

Support

FAQs

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