Submission Details
Severity:
weak source of randomness used which can be used by users / organiser in there favour
Summary
Values used for generating random number are predetermined, which can be used by user or organiser to get the result in there favor.
Vulnerability Details
ChoosingRam contract has two function to select Ram. One is increaseValuesOfParticipants and another is SelectRamIfNotSelected. Both function uses the similar approach to get random.
/// increaseValuesOfParticipants random
uint256 random =
uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao, msg.sender))) % 2;
/// SelectRamIfNotSelected random
uint256 random = uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao))) % ramNFT.tokenCounter();
Where values like block.timestamp, msg.sender, block.prevrandao are being used to generate randomess.
Arbitrum return fixed value of 1 for block.prevrandao, while zksync returns it 2500000000000000.
None of them are random, Using predetermined values like this make it easy to calculate the result beforehead and execute it in there favor.
POC
In existing test suite add following test
////////// USER VERSION ///////////////
function test_weakRandomnessMakeItEasyToKnowWinnerBeforehand() public participants {
vm.startPrank(player1);
/// let's time travel to simulate the winner
/// assume only two players are there who minted nft
/// and not called increase values of participants
vm.warp(1728363903);
choosingRam.increaseValuesOfParticipants(0,1);
choosingRam.increaseValuesOfParticipants(0,1);
choosingRam.increaseValuesOfParticipants(0,1);
choosingRam.increaseValuesOfParticipants(0,1);
choosingRam.increaseValuesOfParticipants(0,1);
console.log("player1:", player1);
console.log("player2:", player2);
console.log("Selected Ram:", choosingRam.selectedRam());
}
//// ORGANISER VERSION ///////
function test_weakRandomnessByOrganiserPickingWinner () public participants{
/// assume organiser want player 1 should win
/// so he will simulate the results before calling the function
/// and goona call it at particular timestamp
vm.startPrank(organiser);
console.log("player1:", player1);
console.log("player2:", player2);
vm.warp(1728691203);
vm.prevrandao(bytes32(uint256(1))); // arbitrum has fixed value of 1
choosingRam.selectRamIfNotSelected();
console.log("Ram will be:", choosingRam.selectedRam());
}
run the following command one by one forge test --mt test_weakRandomnessMakeItEasyToKnowWinnerBeforehand -vv and forge test --mt test_weakRandomnessByOrganiserPickingWinner
it will print the following results, user or organiser can easily simulate the results beforehand and execute things accordingly.
[⠊] Compiling...
No files changed, compilation skipped
Ran 1 test for test/Dussehra.t.sol:CounterTest
[PASS] test_weakRandomnessMakeItEasyToKnowWinnerBeforehand() (gas: 425410)
Logs:
player1: 0x7026B763CBE7d4E72049EA67E89326432a50ef84
player2: 0xEb0A3b7B96C1883858292F0039161abD287E3324
Selected Ram: 0x7026B763CBE7d4E72049EA67E89326432a50ef84
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.59ms (501.58µs CPU time)
[⠊] Compiling...
[⠒] Compiling 46 files with Solc 0.8.20
[⠢] Solc 0.8.20 finished in 2.09s
Compiler run successful!
Ran 1 test for test/Dussehra.t.sol:CounterTest
[PASS] test_weakRandomnessByOrganiserPickingWinner() (gas: 360779)
Logs:
player1: 0x7026B763CBE7d4E72049EA67E89326432a50ef84
player2: 0xEb0A3b7B96C1883858292F0039161abD287E3324
Ram will be: 0x7026B763CBE7d4E72049EA67E89326432a50ef84
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.19ms (466.83µs CPU time)
Impact
Weak Randomness can be utilized to select particular nft as a Ram. Which is not fair with other participants
Tools Used
Manual Review
Recommendations
Consider using chainlink VRF to generate random number as given below. (it's not available on zksync though, works on remaining chains).
I am using chainlink v2.0 as it's available on most chain, v2.5 is still early and available on few chains yet.
using 1.1.1 version of contracts
pragma solidity 0.8.20;
import {RamNFT} from "./RamNFT.sol";
+ import "@chainlink/contracts/src/v0.8/interfaces/VRFCoordinatorV2Interface.sol";
+ import "@chainlink/contracts/src/v0.8/VRFConsumerBaseV2.sol";
- contract ChoosingRam {
+ contract ChoosingRam is VRFConsumerBaseV2 {
error ChoosingRam__InvalidTokenIdOfChallenger();
error ChoosingRam__InvalidTokenIdOfPerticipent();
error ChoosingRam__TimeToBeLikeRamFinish();
error ChoosingRam__CallerIsNotChallenger();
error ChoosingRam__TimeToBeLikeRamIsNotFinish();
error ChoosingRam__EventIsFinished();
+ error ChoosingRam__RandomNumberNotFulfilled();
bool public isRamSelected;
RamNFT public ramNFT;
address public selectedRam;
+ VRFCoordinatorV2Interface COORDINATOR;
+
+ uint64 s_subscriptionId;
+ bytes32 keyHash;
+ uint32 callbackGasLimit;
+ uint16 requestConfirmations;
+ uint32 numWords;
+
+ uint256 public randomValueProvidedByChainlink;
modifier RamIsNotSelected() {
require(!isRamSelected, "Ram is selected!");
_;
}
modifier OnlyOrganiser() {
require(ramNFT.organiser() == msg.sender, "Only organiser can call this function!");
_;
}
- constructor(address _ramNFT) {
+ constructor(
+ address _ramNFT,
+ uint64 subscriptionId,
+ address vrfCoordinator,
+ bytes32 vrfKeyHash,
+ uint32 vrfCallbackGasLimit,
+ uint16 vrfRequestConfirmations,
+ uint32 vrfNumWords
+ )
+ VRFConsumerBaseV2(vrfCoordinator)
+ {
isRamSelected = false;
ramNFT = RamNFT(_ramNFT);
+ COORDINATOR = VRFCoordinatorV2Interface(vrfCoordinator);
+ s_subscriptionId = subscriptionId;
+ keyHash = vrfKeyHash;
+ callbackGasLimit = vrfCallbackGasLimit;
+ requestConfirmations = vrfRequestConfirmations;
+ numWords = vrfNumWords;
}
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 (block.timestamp > 1728691200) {
revert ChoosingRam__TimeToBeLikeRamFinish();
}
+ requestRandomWords();
+ if (randomValueProvidedByChainlink == 0) {
+ revert ChoosingRam__RandomNumberNotFulfilled();
+ }
- uint256 random =
- uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao, msg.sender))) % 2;
+ uint256 random = randomValueProvidedByChainlink % 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(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;
}
}
}
function selectRamIfNotSelected() public RamIsNotSelected OnlyOrganiser {
if (block.timestamp < 1728691200) {
revert ChoosingRam__TimeToBeLikeRamIsNotFinish();
}
if (block.timestamp > 1728777600) {
revert ChoosingRam__EventIsFinished();
}
+ requestRandomWords();
+ if (randomValueProvidedByChainlink == 0) {
+ revert ChoosingRam__RandomNumberNotFulfilled();
+ }
- uint256 random = uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao))) % ramNFT.tokenCounter();
+ uint256 random = randomValueProvidedByChainlink % ramNFT.tokenCounter();
selectedRam = ramNFT.getCharacteristics(random).ram;
isRamSelected = true;
}
+ function requestRandomWords() internal {
+ COORDINATOR.requestRandomWords(
+ keyHash,
+ s_subscriptionId,
+ requestConfirmations,
+ callbackGasLimit,
+ numWords
+ );
+ }
+ function fulfillRandomWords(uint256, uint256[] memory randomWords) internal override {
+ randomValueProvidedByChainlink = randomWords[0];
+ }
}
Updates
Lead Judging Commences
bube about 1 year ago
Submission Judgement Published Assigned finding tags:
Weak randomness in `ChoosingRam::increaseValuesOfParticipants`
Rewards breakdown
nSLOC
218This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.