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

4 stars 3 forks source link

Players have complete freedom to customize the fighter NFT when calling `redeemMintPass` and can redeem fighters of types Dendroid and with rare attributes #366

Open c4-bot-2 opened 7 months ago

c4-bot-2 commented 7 months ago

Lines of code

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

Vulnerability details

The function redeemMintPass allows burning multiple mint passes in exchange for fighters' NFTs. It is mentioned by the sponsor that the player should not have a choice of customizing the fighters' properties and their type. However, nothing prevents a player from:

  1. providing uint8[] fighterTypes of values 1 to mint fighters of types Dendroid.
  2. checking previous transactions in which the dna provided led to minting fighters with rare physical attributes, copying those Dnas and passing them to the redeemMinPass to mint fighters with low rarity attributes. That is because creating physical attributes is deterministic, so providing the same inputs leads to generating a fighter with the same attributes.

    Impact

    This issue has two major impacts:

    • Players with valid mint passes can mint fighters of type Dendroid easily.
    • Players with valid mint passes can mint easily fighters with low rarity attributes which breaks the pseudo-randomness attributes generation aspect

Proof of Concept

For someone having valid mint passes, he calls the function redeemMintPass providing fighterTypes array of values 1. For each mint pass, the inner function _createNewFighter will be called passing the value 1 as fighterType argument which corresponds to Dendroid, a new fighter of type dendroid will be minted for the caller.

function test_redeeming_dendroid_fighters_easily() public {
    uint8[2] memory numToMint = [1, 0];
    bytes memory signature = abi.encodePacked(
        hex"20d5c3e5c6b1457ee95bb5ba0cbf35d70789bad27d94902c67ec738d18f665d84e316edf9b23c154054c7824bba508230449ee98970d7c8b25cc07f3918369481c"
    );
    string[] memory _tokenURIs = new string[](1);
    _tokenURIs[0] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m";

    // first i have to mint an nft from the mintpass contract
    assertEq(_mintPassContract.mintingPaused(), false);
    _mintPassContract.claimMintPass(numToMint, signature, _tokenURIs);
    assertEq(_mintPassContract.balanceOf(_ownerAddress), 1);
    assertEq(_mintPassContract.ownerOf(1), _ownerAddress);

    // once owning one i can then redeem it for a fighter
    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 Notice that I can provide value 1 which corresponds to Dendroid type
    _neuralNetHashes[0] = "neuralnethash";
    _modelTypes[0] = "original";
    _iconsTypes[0] = 1;

    // approve the fighterfarm contract to burn the mintpass
    _mintPassContract.approve(address(_fighterFarmContract), 1);

    _fighterFarmContract.redeemMintPass(
    _mintpassIdsToBurn, _fighterTypes, _iconsTypes, _mintPassDNAs, _neuralNetHashes, _modelTypes
    );

    // check balance to see if we successfully redeemed the mintpass for a fighter
    assertEq(_fighterFarmContract.balanceOf(_ownerAddress), 1);
}
Ran 1 test for test/FighterFarm.t.sol:FighterFarmTest
[PASS] test_redeeming_dendroid_fighters_easily() (gas: 578678)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 6.56ms

Ran 1 test suite: 1 tests passed, 0 failed, 0 skipped (1 total tests)

The player can also inspect previous transactions that minted a fighter with rare attributes, copy the provided mintPassDnas and provide them as argument in the redeemMintPass. The _createNewFighter function calls AiArenaHelper to create the physical attributes for the fighter. The probability of attributes is deterministic and since the player provided dna that already led to a fighter with rare attributes, his fighter will also have rare attributes.

Tools Used

Manual Review

Recommended Mitigation Steps

The main issue is that the mint pass token is not tied to the fighter properties that the player should claim and the player has complete freedom of the inputs. Consider implementing a signature mechanism that prevents the player from changing the fighter's properties like implemented in claimFighters

Assessed type

Invalid Validation

c4-pre-sort commented 7 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 7 months ago

raymondfam marked the issue as duplicate of #33

c4-pre-sort commented 6 months ago

raymondfam marked the issue as duplicate of #1626

c4-judge commented 6 months ago

HickupHH3 marked the issue as satisfactory

c4-judge commented 6 months ago

HickupHH3 marked the issue as selected for report

SonnyCastro commented 6 months ago

Mitigated here

c4-sponsor commented 5 months ago

brandinho (sponsor) confirmed