AI Arena - taner2344's results

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/MergingPool.sol#L139-L167

Vulnerability details

Impact

Players will not be able to receive the NFTs that they won.

Proof of Concept

in FighterFarm contract declares that MAX_FIGHTERS_ALLOWED = 10; so , If a successful player participates in a round with different NFTs and earns enough points to win a new NFT, they will not be able to receive the NFTs they won, and the transaction will revert()

Here is scenario :

• Bob has 9 fighter NFT's

• Bob is very successful player and he takes first two place with his two different NFT's in one round.

• at the end of the round, contract owner calls MergingPool::pickWinner(uint256[] calldata winners) and passes Bob's NFT's as a parameter.

• Bob calls MergingPool::claimRewards and as a expected transaction reverts. He couldn't get any NFT because he has exceeding 10 NFT for per address rule.

• Bob has lost his 10th NFT’s even though it is allowed to have it...

• Bob owns 9 fighter NFTs.

• Bob is successful player, Bob secured the first two positions in a round using two different NFTs.

• At the round's end, the contract owner calls MergingPool::pickWinner(uint256[] calldata winners) , including Bob's NFTs as parameters.

• Bob calls MergingPool::claimRewards , but the transaction reverts as expected. He is not able to claim any additional NFTs, he remains capped at the 10 NFT per address limit.

• Bob loses his NFT's even though possessing it initially violates the rule.

The issue was classified as High Risk because there is a risk of losing won NFTs and players are experiencing losses.

Here is POC ...

Please add following code to test/MergingPool.t.sol

function testPickWinnerTaner() public {
        //Bob has 9 fighter to owner
        _mintFromMergingPool(_ownerAddress);
        _mintFromMergingPool(_ownerAddress);
        _mintFromMergingPool(_ownerAddress);
        _mintFromMergingPool(_ownerAddress);
        _mintFromMergingPool(_ownerAddress);
        _mintFromMergingPool(_ownerAddress);
        _mintFromMergingPool(_ownerAddress);
        _mintFromMergingPool(_ownerAddress);
        _mintFromMergingPool(_ownerAddress);

        assertEq(_fighterFarmContract.doesTokenExist(0), true);
        assertEq(_fighterFarmContract.doesTokenExist(1), true);
        assertEq(_fighterFarmContract.doesTokenExist(2), true);
        assertEq(_fighterFarmContract.doesTokenExist(3), true);
        assertEq(_fighterFarmContract.doesTokenExist(4), true);
        assertEq(_fighterFarmContract.doesTokenExist(5), true);
        assertEq(_fighterFarmContract.doesTokenExist(6), true);
        assertEq(_fighterFarmContract.doesTokenExist(7), true);
        assertEq(_fighterFarmContract.doesTokenExist(8), true);


       // Bob goes the battle with two different fighters :fighter number 5 and 7 ...
       // and win the first two titles with his fighters.
        uint256[] memory _winners = new uint256[](2);
        _winners[0] = 5; //Bob's fighter number 5
        _winners[1] = 7; //Bob's fighter number 7

        _mergingPoolContract.pickWinner(_winners); //contract owner picking winners and passing Bob's fighters

        assertEq(_mergingPoolContract.isSelectionComplete(0), true);
        assertEq(_mergingPoolContract.winnerAddresses(0, 0) == _ownerAddress, true); //first winner is Bob
        assertEq(_mergingPoolContract.winnerAddresses(0, 1) == _ownerAddress, true); //second winner is also Bob

        //preparing reward NFT's
         string[] memory _modelURIs = new string[](2);
        _modelURIs[0] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m";
        _modelURIs[1] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m";
        string[] memory _modelTypes = new string[](2);
        _modelTypes[0] = "original";
        _modelTypes[1] = "original";
        uint256[2][] memory _customAttributes = new uint256[2][](2);
        _customAttributes[0][0] = uint256(1);
        _customAttributes[0][1] = uint256(80);
        _customAttributes[1][0] = uint256(1);
        _customAttributes[1][1] = uint256(80);

        // Bob is trying to claiming rewards but transaction reverts
         vm.expectRevert();
        _mergingPoolContract.claimRewards(_modelURIs, _modelTypes, _customAttributes);
    }

Tools Used

Manual review, Foundry

Recommended Mitigation Steps

Please consider to add checking winning address logic to game.

Assessed type

Context