AI Arena - Breeje's results

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/GameItems.sol#L301

Vulnerability details

Impact

Anyone can transfer the GameItems when they are not supposed to, breaks the invariant.

Proof of Concept

GameItems NFT which has transferable attribute set to false are supposed to be not transferable.

To implement it, GameItems currently overrides the safeTransferFrom function and adds a require check to make sure that transferable is set to true. If not, it will revert and not allow transfers.

    function safeTransferFrom(
        address from, 
        address to, 
        uint256 tokenId,
        uint256 amount,
        bytes memory data
    ) 
        public 
        override(ERC1155)
    {
        require(allGameItemAttributes[tokenId].transferable);
        super.safeTransferFrom(from, to, tokenId, amount, data);
    }

But in ERC1155 of openzeppelin, there are couple ways to transfer the NFTs:

1. Using safeTransferFrom function: Which is correctly override to assert the transferable condition.

2. Using safeBatchTransferFrom function: This one is not override in GameItems and can be used by anyone to transfer the NFTs.

Hence, anyone can use safeBatchTransferFrom function to transfer the non transferable NFTs.

Tools Used

VS Code

Recommended Mitigation Steps

Override the safeBatchTransferFrom implementation as well in GameItems just like how it is done for safeTransferFrom to ensure NFTs are not transferable if transferable attribute is set to false.

+   function safeBatchTransferFrom(
+       address from,
+       address to,
+       uint256[] memory ids,
+       uint256[] memory amounts,
+       bytes memory data
+   ) 
+       public 
+       override(ERC1155)
+   {
+       require(allGameItemAttributes[tokenId].transferable);
+       super.safeBatchTransferFrom(from, to, ids, amounts, data);
+   }

Assessed type

Token-Transfer