code-423n4/2024-02-ai-arena-findings

Users with minting passes can mint fighters that are both Dendroids and Icons

c4-bot-6 opened this issue · 5 comments

Lines of code

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

Vulnerability details

Impact

There are two types of fighters: Champions and Dendroids.

Icons are a rare subtype of champions. On the other hand Dendroids are a are a more exclusive class of NFTs, which are rarer than champions.

But it is possible to mint a Dendroid that is also an Icon. This breaks the previously mentioned invariant that an icon is a subtype of champion.

NFTs are valued by their rarity and special traits. In this case, it is possible to mint one with attributes that should not co-exist, which can lead to an unfair pricing in markets, and potential glitches on the game given the unexpected combination. A Medium severity was assessed given the broken invariant and the given reasons.

Vulnerability Details

Users can mint new fighters via redeemMintPass().

They can provide both the fighterType, and the iconsType.

Those values aren't validated in the function, nor on any other internal call (as proved on the POC). That means it is possible to create a Champion (fighterType == 0) that is also an Icon (iconsType > 0).

Reference:

iconsType Type of icons fighter (0 means it's not an icon).

Proof of Concept

  • Add the test to test/FighterFarm.t.sol
  • Run forge test --mt "testRedeemMintPassIcons"
function testRedeemMintPassDendroidIcon() public {
    // Claim mint pass and approve it to _fighterFarmContract
    uint8[2] memory numToMint = [1, 0];
    bytes memory signature = abi.encodePacked(hex"20d5c3e5c6b1457ee95bb5ba0cbf35d70789bad27d94902c67ec738d18f665d84e316edf9b23c154054c7824bba508230449ee98970d7c8b25cc07f3918369481c");
    string[] memory _tokenURIs = new string[](1);
    _tokenURIs[0] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m";
    _mintPassContract.claimMintPass(numToMint, signature, _tokenURIs);
    _mintPassContract.approve(address(_fighterFarmContract), 1);

    // Build parameters to pass to redeemMintPass() 
    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;                   // @audit Dendroid
    _neuralNetHashes[0] = "neuralnethash";
    _modelTypes[0] = "original";
    _iconsTypes[0] = 1;                   // @audit Icon

    // The fighter was successfully minted as a Dendroid and Icon
    _fighterFarmContract.redeemMintPass(
        _mintpassIdsToBurn, _fighterTypes, _iconsTypes, _mintPassDNAs, _neuralNetHashes, _modelTypes
    );

    // Verify that the fighter is both a Dendroid and an Icon
    (,,,,,,, uint8 iconsType, bool dendroidBool) = _fighterFarmContract.fighters(0);
    assertEq(dendroidBool, true);
    assertEq(iconsType, 1);
}

Recommended Mitigation Steps

Validate that the minted fighter can't be a Dendroid and an Icon at the same time:

+    if (fighterType == 1 && iconsType > 0) {
+        revert();
+    }
  • fighterType == 1 => Dendroid
  • iconsType > 0 => Icon

Assessed type

Invalid Validation

raymondfam marked the issue as sufficient quality report

raymondfam marked the issue as duplicate of #33

raymondfam marked the issue as duplicate of #1626

HickupHH3 changed the severity to 3 (High Risk)

HickupHH3 marked the issue as satisfactory