First Flight #10: One Shot

Summary

In RapBattle :: _battle() function , there is no update of status "battles_won" when the user wins the battle. This can lead to incorrect information about the user.

Vulnerability Details

In RapBattle :: _battle() function there is no status updation even after the user won the battle.

Impact

The impact is that even after winning we check for our status it always shows that battles_won is zero. This can lead to misrepresentation of the user.

Code Snippet

    function _battle(uint256 _tokenId, uint256 _credBet) internal {
    address _defender = defender;
    require(defenderBet == _credBet, "RapBattle: Bet amounts do not match");
    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;

    // Reset the defender
    defender = address(0);
    emit Battle(msg.sender, _tokenId, random < defenderRapperSkill ? _defender : msg.sender);

    // If random <= defenderRapperSkill -> defenderRapperSkill wins, otherwise they lose
    if (random <= defenderRapperSkill) {
        // We give them the money the defender deposited, and the challenger's bet
     // @audit : status for battles_won is not updated .
        credToken.transfer(_defender, defenderBet);
        credToken.transferFrom(msg.sender, _defender, _credBet);
    } else {
        // Otherwise, since the challenger never sent us the money, we just give the money in the contract
        credToken.transfer(msg.sender, _credBet);
    }

    totalPrize = 0;
    // Return the defender's NFT
    oneShotNft.transferFrom(address(this), _defender, defenderTokenId);
}

POC

    modifier twoSkilledRappers() {
    vm.startPrank(user);
    oneShot.mintRapper();
    oneShot.approve(address(streets), 0);
    streets.stake(0);
    vm.stopPrank();

    vm.startPrank(challenger);
    oneShot.mintRapper();
    oneShot.approve(address(streets), 1);
    streets.stake(1);
    vm.stopPrank();

    vm.warp(4 days + 1);

    vm.startPrank(user);
    streets.unstake(0);
    vm.stopPrank();
    vm.startPrank(challenger);
    streets.unstake(1);
    vm.stopPrank();
    _;
}

     function testWinnerTransferredBetAmount(uint256 randomBlock) public twoSkilledRappers {
    vm.startPrank(user);
    oneShot.approve(address(rapBattle), 0);
    cred.approve(address(rapBattle), 3);
    console.log("User allowance before battle:", cred.allowance(user, address(rapBattle)));
    rapBattle.goOnStageOrBattle(0, 3);
    vm.stopPrank();

    vm.startPrank(challenger);
    oneShot.approve(address(rapBattle), 1);
    cred.approve(address(rapBattle), 3);
    console.log("User allowance before battle:", cred.allowance(challenger, address(rapBattle)));

    // Change the block number so we get different RNG
    vm.roll(randomBlock);
    vm.recordLogs();
    rapBattle.goOnStageOrBattle(1, 3);
    vm.stopPrank();

    Vm.Log[] memory entries = vm.getRecordedLogs();
    // Convert the event bytes32 objects -> address
    address winner = address(uint160(uint256(entries[0].topics[2])));
    assert(cred.balanceOf(winner) == 7);
    console.log("winner is" , winner);
    vm.startPrank(challenger);
    stats = oneShot.getRapperStats(1);
    vm.stopPrank();
    console.log("battlesWon by the Slim Shady is" , stats.battlesWon);
    }

Tools Used

Manual check

Recommendations

It recommended to update the status of the user after the battle is won .Modify this function so that RapBattle.sol can also call this function and pass
the updated value of battlesWon there . So that when user check their status it will show the updated status.

Modify the OneShot.sol contract functions like this .

      modifier onlylimitedContract()  {
       require((msg.sender == address(_streetsContract)) || (msg.sender == address(_rapContract)), "Not the choosed contract");
        _;
       }
       function updateRapperStats(                       
    uint256 tokenId,
    bool weakKnees,
    bool heavyArms,
    bool spaghettiSweater,
    bool calmAndReady,
    uint256 battlesWon
) public onlylimitedContract{ 
    RapperStats storage metadata = rapperStats[tokenId];
    metadata.weakKnees = weakKnees;
    metadata.heavyArms = heavyArms;
    metadata.spaghettiSweater = spaghettiSweater;
    metadata.calmAndReady = calmAndReady;
    metadata.battlesWon = battlesWon;
}

Modify the RapBattle.sol functions like this

    mapping(address => uint256) battles_won ;

    function _battle(uint256 _tokenId, uint256 _credBet) internal {
    address _defender = defender;
    require(defenderBet == _credBet, "RapBattle: Bet amounts do not match");
    uint256 defenderRapperSkill = getRapperSkill(defenderTokenId);
    uint256 challengerRapperSkill = getRapperSkill(_tokenId);
    uint256 totalBattleSkill = defenderRapperSkill + challengerRapperSkill;
    uint256 totalPrize = defenderBet + _credBet;
    console.log("totalPrize for winner is",totalPrize);
    uint256 randomvalue = uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao, msg.sender)));
    uint256 random =
        uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao, msg.sender))) % totalBattleSkill;
    // Reset the defender
    defender = address(0);
    //@audit-issue -4 : battles won is not updated anywhere -- valid
    emit Battle(msg.sender, _tokenId, random < defenderRapperSkill ? _defender : msg.sender);
    if (random <= defenderRapperSkill) {
        // We give them the money the defender deposited, and the challenger's bet
        credToken.transfer(_defender, defenderBet);
        credToken.transferFrom(msg.sender, _defender, _credBet);
        battles_won[_defender]++;
        updateRapperstatus(defenderTokenId);
    } else {
        // Otherwise, since the challenger never sent us the money, we just give the money in the contract
        credToken.transfer(msg.sender, _credBet);
         battles_won[msg.sender]++;
         updateRapperstatus(_tokenId);
    
    }
       totalPrize = 0;
    // Return the defender's NFT
    oneShotNft.transferFrom(address(this), _defender, defenderTokenId);
}

 function updateRapperstatus(uint256 _tokenId) public 
    {
    IOneShot.RapperStats memory stats = oneShotNft.getRapperStats(_tokenId);
     oneShotNft.updateRapperStats(
        _tokenId,
        stats.weakKnees,
        stats.heavyArms,
        stats.spaghettiSweater,
        stats.calmAndReady,
        battles_won[msg.sender]
    );
}