Security Snippets: #3

AI Arena is a PvP platform fighting game where the fighters are AIs that were trained by humans. In the web3 version of our game, these fighters are tokenized via the FighterFarm.sol smart contract.

In FighterFarm.sol there is a mapping numElements which stores the number of possible types of elements a fighter can have in a generation:

    /// @notice Mapping of number elements by generation.
    mapping(uint8 => uint8) public numElements;

But the problem here is that only the initial generation, Generation 0, is initialized to 3, in the numElements mapping during the constructor of FighterFarm.sol.

    constructor(address ownerAddress, address delegatedAddress, address treasuryAddress_)
        ERC721("AI Arena Fighter", "FTR")
    {
        _ownerAddress = ownerAddress;
        _delegatedAddress = delegatedAddress;
        treasuryAddress = treasuryAddress_;
        numElements[0] = 3;
    } 

It is therefore not possible to write to the numElements mapping for any other generations. As they are uninitialized, numElements[i] = 0 when i != 0

Moreover, this numElements mapping is read from when creating a fighter.

    function _createFighterBase(
        uint256 dna, 
        uint8 fighterType
    ) 
        private 
        view 
        returns (uint256, uint256, uint256) 
    {
=>      uint256 element = dna % numElements[generation[fighterType]]; // numElements is 0 when generation[fighterType] != 0.
        uint256 weight = dna % 31 + 65;
        uint256 newDna = fighterType == 0 ? dna : uint256(fighterType);
        return (element, weight, newDna);
    }

Therefore if the protocol updates to a new generation of fighters, it will not be able to create anymore new fighters as numElements[generation[fighterType]] will be uninitialized and therefore equal 0. This will cause the transaction to always revert as any modulo by 0 will cause a panic.

As mitigation recomendation was to let the admin to update the numElements mapping when a new generation is created.