H-03 MitigationConfirmed

[c4-bot-5]

Lines of code

Vulnerability details

Lines of code

Old lines of code

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

Mitigated lines of code

https://github.com/ArenaX-Labs/2024-02-ai-arena-mitigation/blob/fix-47/src/FighterFarm.sol#L261-L304

Vulnerability description

The issue was reported in #366.

FighterFarm implements a method to redeem mint passes and obtain NFT fighters. This method is redeemMintPass:

233      function redeemMintPass(
234          uint256[] calldata mintpassIdsToBurn,
235          uint8[] calldata fighterTypes,
236          uint8[] calldata iconsTypes,
237          string[] calldata mintPassDnas,
238          string[] calldata modelHashes,
239          string[] calldata modelTypes
240      ) 
241          external 
242      {
243          require(
244              mintpassIdsToBurn.length == mintPassDnas.length && 
245              mintPassDnas.length == fighterTypes.length && 
246              fighterTypes.length == modelHashes.length &&
247              modelHashes.length == modelTypes.length
248          );
249          for (uint16 i = 0; i < mintpassIdsToBurn.length; i++) {
250              require(msg.sender == _mintpassInstance.ownerOf(mintpassIdsToBurn[i]));
251              _mintpassInstance.burn(mintpassIdsToBurn[i]);
252              _createNewFighter(
253                  msg.sender, 
254                  uint256(keccak256(abi.encode(mintPassDnas[i]))), 
255                  modelHashes[i], 
256                  modelTypes[i],
257                  fighterTypes[i],
258                  iconsTypes[i],
259                  [uint256(100), uint256(100)]
260              );
261          }
262      }

This method can be directly called by a player, who could create new fighters according to passed parameters. This means that a player can find and provide dna to mint fighters with very rare physical attributes and can easily mint fighters of type Dendroid. This issue is due to a lack of validation of redeem passes.

Recommended Mitigation proposed by wardens

The proposed mitigation is to apply the same validation check that is in claimFighters. However, the signature is applied on different parameters:

FighterFarm.sol#L280-L289

280           bytes32 msgHash = bytes32(keccak256(abi.encode(
281            mintpassIdsToBurn,
282            fighterTypes,
283            iconsTypes,
284            mintPassDnas,
285            modelHashes,
286            modelTypes
287          )));
288  
289          require(Verification.verify(_delegatedAddress, msgHash, signature));

According to what developers explain to me, the backend server assigns metadata to a specific mintPassId. This means that a specific tuple of (mintpassIdsToBurn, fighterTypes, iconsTypes, mintPassDnas, modelHashes, modelTypes) is valid only if the player has the signature of that tuple obtained by the backend server.

This solution was implemented by the Ai Arena team

Comment about the Mitigation Proposal

We think this approach could lead to several issues. It mitigates the issue described in issue 366, but strongly modifies the nature and usage of mint passes.

We report our comments below.

The signature is the same for several mint passes

Let's think Bob has several mint passes. To create fighters, he has to ask to backend server to obtain the signature. If he asks for the signature for a subset of those mint passes, he has to redeem those mint passes together. In other words, the signature is not one for each mintpassIdToBurn. We suggest having a signature for each mintpassIdToBurn. There is another issue. We don't know how the signature is generated by the backend. However, let's think a player asks the signature to redeem 11 mint passes he/she owns. This signature will never work:

• The player calls redeemMintPass with the right signature, passing mintpassIdsToBurn with 11 elements.
• redeemMintPass tries to call _createNewFighter 11 times.
• Assuming at the beginning the player doesn't own any fighter, _createNewFighter will fail at the 11th iteration, making the transaction revert, because the line FighterFarm.sol#L540

So, even if the player obtained a valid signature, he/she can not use it.

The dna is part of mint pass metadata. So, it is possible to forecast the attributes of the redeemed fighters

Mint passes can be bought, for example, on OpenSea. The Ai Champion Mint Pass 245 can be bought for a price of 2,888 ETH, to date. We can see that its dna is 0x34d5c19afa611134f0c27b7b6d09e34c2d579f48d7b92e8c8f997af4981469fd. We can forecast the physical attributes of the fighter that can be redeemed using this mint pass. Using this code:

function testGetAttributes() public {
    uint256 newDna = uint256(keccak256(abi.encode("0x34d5c19afa611134f0c27b7b6d09e34c2d579f48d7b92e8c8f997af4981469fd")));

    // Assuming generation = 0, iconsType=1, fighterType = 0
    uint256 generation = 0;
    uint256 iconsType = 1;
    uint256 fighterType = 0;
    bool dendroidBool = fighterType == 1;

    FighterOps.FighterPhysicalAttributes memory attrs = _helperContract.createPhysicalAttributes(
        newDna,
        0,
        1,
        false
    );
    console.log(attrs.head); // head = 1
    console.log(attrs.eyes); // eyes = 50
    console.log(attrs.mouth); // mouth = 3
    console.log(attrs.body); // body = 3
    console.log(attrs.hands); // hands = 2
    console.log(attrs.feet); // feet = 2
}

We asked to sponsor and it seems the wanted behavior. They want the outcoming physical attributes to be foreseeable. Furthermore, they wanted that the mechanism to compute these attributes from dna is on-chain. However, we propose to move this mechanism off-chain. Because of the way the dna is computed by the backend server, the fact physical attributes are computed on-chain doesn't add transparency to the operation. It could be better if the mint pass already shows the physical attributes of the outcoming fighter. We want to add that also the outcomes of the reRoll operation are foreseeable. In other words, players can forecast before buying what physical attributes could be reached using the reRoll operation.

Conclusion

We think this mitigation works to prevent players from creating fighters with wanted attributes. Now, the traits are bound to dna written in the mintPass, which is defined by the backend server. This value is known and permits forecasting future attributes of a fighter before buying the mintPass. This issue could be related to other medium issues reported during the contest, like issue #1017 - Users can get benefited from DNA pseudorandomly calculation. However, according to my conversation with the sponsor during this mitigation review, it seems that the possibility to forecast fighter traits is a wanted behavior.

[c4-judge]

jhsagd76 marked the issue as satisfactory

[c4-judge]

jhsagd76 marked the issue as confirmed for report

[brandinho]

To address the comment about a user possibly trying to mint 11 mintpasses - this will never happen. We limit 10 at a time, and generate signatures accordingly.