Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: high
Invalid

`selectRamIfNotSelected()` doesn't check if an address was Ram Before

Summary

There is no check if an address was Ram before in selectRamIfNotSelected()

Vulnerability Details

The documentation said that in choosingRam.sol exist a function where user can be ram and can be selected as ram as long they have not been ram before. However in the organizer-controlled function choosingRam.sol::selectRamIfNotSelected() there is no valid checking for that.

function selectRamIfNotSelected() public RamIsNotSelected OnlyOrganiser {
if (block.timestamp < 1728691200) {
revert ChoosingRam__TimeToBeLikeRamIsNotFinish();
}
if (block.timestamp > 1728777600) {
revert ChoosingRam__EventIsFinished();
}
uint256 random = uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao))) % ramNFT.tokenCounter();
selectedRam = ramNFT.getCharacteristics(random).ram;
isRamSelected = true;
}

Impact

a Ram that has previously chosen could potentially get chosen again.

Tools Used

Manual Analysis

Recommendations

Developer can try to add a mapping, when a user is successfully updated and gain ram privilege which will be stored in selectedRam variable in this function increaseValuesOfParticipants(), it is also update a mapping like this

mapping(address => bool) public wasRam;

let's say address X is chosen as Ram upon successfully update his status, the last else if check should be modified into this

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;
+ wasRam[ramNFT.getCharacteristics(tokenIdOfChallenger).ram] = true;
}

and can add a check for the selectRamIfNotSelected() function

function selectRamIfNotSelected() public RamIsNotSelected OnlyOrganiser {
if (block.timestamp < 1728691200) {
revert ChoosingRam__TimeToBeLikeRamIsNotFinish();
}
if (block.timestamp > 1728777600) {
revert ChoosingRam__EventIsFinished();
}
+ if(wasRam[ramNFT.getCharacteristics(random).ram] == true){
+ revert AlreadyRamBefore();
+ }
uint256 random = uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao))) % ramNFT.tokenCounter();
selectedRam = ramNFT.getCharacteristics(random).ram;
isRamSelected = true;
}

NOTE: AlreadyRamBefore() can be a new revert

Updates

Lead Judging Commences

bube Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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