Summary
The ChoosingRam::increaseValuesOfParticipants function does not properly set the isRamSelected flag to true when a Ram is selected. This allows the selectedRam variable to be overwritten multiple times, leading to inconsistencies and potential manipulation. As a result, the Dussehra::killRavana function cannot be executed correctly because it requires isRamSelected to be true.
Vulnerability Details
function increaseValuesOfParticipants(uint256 tokenIdOfChallenger, uint256 tokenIdOfAnyPerticipent)
public
RamIsNotSelected
{
.
.
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;
}
}
}
ChoosingRam::increaseValuesOfParticipants function allows to increase value of participants. If specific NFT gains so much value it will be selected as Ram, address of owner will be stored in selectedRam. Problem arises because isRamSelected is not set to true, which represents if Ram is actually selected.
Impact
Due to vulnerability, selected Ram via increaseValuesOfParticipants function will not be actually selected as Ram, which is not desired. Because Dussehra::killRavana function to be executed, isRamSelected needs to be set to true. Organiser would still need to pick Ram via selectRamIfNotSelected function, and it will be random pick.
Also it will be possible to overwrite selectedRam variable by calling increaseValuesOfParticipants function again.
Tools Used
Manual review
Recommendations
Set isRamSelected to true when Ram is selected.
function increaseValuesOfParticipants(uint256 tokenIdOfChallenger, uint256 tokenIdOfAnyPerticipent)
public
RamIsNotSelected
{
.
.
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;
+ isRamSelected = true;
}
} 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;
+ isRamSelected = true;
}
}
