AI Arena - pontifex's results

Lines of code

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

Vulnerability details

Impact

Users can't re-roll fighters with tokenId more than type(uint8).max but should be able. It is a medium issue because an important part of the payable functionality of the protocol does not work properly.

Proof of Concept

Users should be able to re-roll their fighters at least maxRerollsAllowed[fighterType] times via the FighterFarm.reRoll for the rerollCost amount of NRN tokens:

    /// @notice Rolls a new fighter with random traits.
    /// @param tokenId ID of the fighter being re-rolled.
    /// @param fighterType The fighter type.
    function reRoll(uint8 tokenId, uint8 fighterType) public {
        require(msg.sender == ownerOf(tokenId));
        require(numRerolls[tokenId] < maxRerollsAllowed[fighterType]);
        require(_neuronInstance.balanceOf(msg.sender) >= rerollCost, "Not enough NRN for reroll");

But the max tokenId which the reRoll function can accept is type(uint8).max. So fighters with tokenId more than type(uint8).max can't be re-rolled.

Tools Used

Manual review

Recommended Mitigation Steps

Consider using type uint256 instead of uint8:

-   function reRoll(uint8 tokenId, uint8 fighterType) public {
+   function reRoll(uint256 tokenId, uint8 fighterType) public {

Assessed type

Error

Lines of code

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

Vulnerability details

Impact

In case the amount of unclaimed rewards for the user exceeds MAX_FIGHTERS_ALLOWED the user's rewards will be blocked permanently. The user won't be able receives rewards at all.

Proof of Concept

Users are allowed to claim butch rewards for multiple rounds using the MergingPool.claimRewards function but they can't specify the number of round or reward to be claimed:

    function claimRewards(
        string[] calldata modelURIs,
        string[] calldata modelTypes,
        uint256[2][] calldata customAttributes
    )
        external
    {
        uint256 winnersLength;
        uint32 claimIndex = 0;
        uint32 lowerBound = numRoundsClaimed[msg.sender];
        for (uint32 currentRound = lowerBound; currentRound < roundId; currentRound++) {
            numRoundsClaimed[msg.sender] += 1;
            winnersLength = winnerAddresses[currentRound].length;
            for (uint32 j = 0; j < winnersLength; j++) {
                if (msg.sender == winnerAddresses[currentRound][j]) {
                    _fighterFarmInstance.mintFromMergingPool(            

There is also no function for claiming rewards for a specified round at the contract. In case the amount of rewards for the user exceeds the MAX_FIGHTERS_ALLOWED the user won't be able to claim rewards at all because of revert in the FighterFarm._createNewFighter function:

    function mintFromMergingPool(
        address to,
        string calldata modelHash,
        string calldata modelType,
        uint256[2] calldata customAttributes
    )
        public
    {
        require(msg.sender == _mergingPoolAddress);
        _createNewFighter(

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

    function _createNewFighter(
        address to,
        uint256 dna,
        string memory modelHash,
        string memory modelType,
        uint8 fighterType,
        uint8 iconsType,
        uint256[2] memory customAttributes
    )
        private
    {  
        require(balanceOf(to) < MAX_FIGHTERS_ALLOWED);

Even if the user transfers all his fighters to another address it will still be impossible to claim rewards. It is also inpossible to change the winnerAddresses mapping for previous rounds because the MergingPool.pickWinner function allows the admin to pick the winners only for the current round: https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/MergingPool.sol#L111-L118

Tools Used

Manual review

Recommended Mitigation Steps

Consider checking the acceptable amount of rewards which user can receive and bound the for loop with the amount:

    function claimRewards(
        string[] calldata modelURIs,
        string[] calldata modelTypes,
        uint256[2][] calldata customAttributes
    )
        external
    {
        uint256 winnersLength;
        uint32 claimIndex = 0;
+       uint256 claimable = _fighterFarmInstance.MAX_FIGHTERS_ALLOWED - _fighterFarmInstance.balanceOf(msg.sender);
        uint32 lowerBound = numRoundsClaimed[msg.sender];
-       for (uint32 currentRound = lowerBound; currentRound < roundId; currentRound++) {
+       for (uint32 currentRound = lowerBound; currentRound < roundId && claimIndex < claimable; currentRound++) {
            numRoundsClaimed[msg.sender] += 1;
            winnersLength = winnerAddresses[currentRound].length;
            for (uint32 j = 0; j < winnersLength; j++) {
                if (msg.sender == winnerAddresses[currentRound][j]) {
                    _fighterFarmInstance.mintFromMergingPool(     

Assessed type

Loop