AI Arena - 0xAleko's results

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/1d18d1298729e443e14fea08149c77182a65da32/src/FighterFarm.sol#L372

Vulnerability details

Impact

A user can bypass the maxRerollsAllowed requirement by using a different fighterType instead of his fighters, if he has enough NRC for pay the reroll cost.

Proof of Concept

add some changes in testReroll.

1. A gift to the user, sufficient for the cost of a neuron token reroll.

2. When using merging pool minting as we know, fighter types will be 0. https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L322-L329

3. reRoll our nft fighter using fighterType 1, 3x times because maxReroll defaults is 3 and after change fighterType to 0. This way we can do it multiple times and bypass the maxRerollsAllowed check.

    function testRerollAnotherFighterType() public {
        _mintFromMergingPool(_ownerAddress);
        // get 4k neuron from treasury

        vm.prank(_treasuryAddress);
        _neuronContract.transfer(_ownerAddress, 10_000 * 10 ** 18);
        assertEq(10_000 * 10 ** 18 == _neuronContract.balanceOf(_ownerAddress), true);

        // after successfully minting a fighter, update the model
        if (_fighterFarmContract.ownerOf(0) == _ownerAddress) {
            uint256 neuronBalanceBeforeReRoll = _neuronContract.balanceOf(_ownerAddress);
            uint8 tokenId = 0;
            uint8 fighterType = 1;

            _neuronContract.addSpender(address(_fighterFarmContract));

            _fighterFarmContract.reRoll(tokenId, fighterType);
            _fighterFarmContract.reRoll(tokenId, fighterType);
            _fighterFarmContract.reRoll(tokenId, fighterType);

            fighterType = 0;
            _fighterFarmContract.reRoll(tokenId, fighterType);
            _fighterFarmContract.reRoll(tokenId, fighterType);
            _fighterFarmContract.reRoll(tokenId, fighterType);

            assertEq(_fighterFarmContract.numRerolls(0), 6);
            assertEq(neuronBalanceBeforeReRoll > _neuronContract.balanceOf(_ownerAddress), true);
        }
    }

Tools Used

Manual Review, Foundry.

Recommended Mitigation Steps

require(numRerolls[tokenId] < maxRerollsAllowed[fighterType]); - add check for NFTs fighter type.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

If a user already has, for example, 9 fighters and the MAX_FIGHTERS_ALLOWED=10, their transaction to claim a 2 fighters reward will be reverted in claimRewards() if they are selected as the winner in pickWinner(), because the limit is 10. Consequently, they cannot mint even 1 additional fighter.

The user can still claim part of the reward. However, due to the function's all-or-nothing design, they must either claim the entire reward or none of it.

Proof of Concept

1. first of all mint 9 fighters.
2. call pickWinner() function.
3. make sure the result of getUnclaimedRewards() is 2.
4. as _ownerAddress - owner of fighters, try to claimRewards.

  function testClaimRewardsMaxAllowedFighter() public {
        /// mint 9 fighter for owner - less than max allowed count of fighters
        _mintFromMergingPool(_ownerAddress);
        _mintFromMergingPool(_ownerAddress);
        _mintFromMergingPool(_ownerAddress);
        _mintFromMergingPool(_ownerAddress);
        _mintFromMergingPool(_ownerAddress);
        _mintFromMergingPool(_ownerAddress);
        _mintFromMergingPool(_ownerAddress);
        _mintFromMergingPool(_ownerAddress);
        _mintFromMergingPool(_ownerAddress);

        uint256[] memory _winners = new uint256[](2);
        _winners[0] = 0;
        _winners[1] = 1;

        _mergingPoolContract.pickWinner(_winners);

        uint256 numRewards = _mergingPoolContract.getUnclaimedRewards(_ownerAddress);
        assertEq(numRewards, 2);

        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);

        vm.prank(_ownerAddress);
        vm.expectRevert(); // expect revert from mintFromMergingPool() - `require(balanceOf(to) < MAX_FIGHTERS_ALLOWED);`
        _mergingPoolContract.claimRewards(_modelURIs, _modelTypes, _customAttributes);
    }

Tools Used

Manual review, foundry.

Recommended Mitigation Steps

Refactor claimRewards() function, so that winner can claim part of the winnings if he can't claim all.

Assessed type

Invalid Validation