code-423n4 / 2023-01-timeswap-findings

0 stars 0 forks source link

Incorrect `totalSupply()` function design #263

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-timeswap/blob/main/packages/v2-token/src/base/ERC1155Enumerable.sol#L36-L37

Vulnerability details

Impact

In ERC1155Enumerable.sol#L36-L37 line, totalsuppyl() of ERC1155 is calculated

packages/v2-token/src/base/ERC1155Enumerable.sol:
  34  
  35:     /// @inheritdoc IERC1155Enumerable
  36:     function totalSupply() public view override returns (uint256) {
  37:         return _allTokens.length;
  38:     }

However, this design is of ERC777, not ERC1155

ERC721 is suitable for one-of-a-kind NFTs while ERC1155 is suitable for multiple assets that can be combined and divided

The correct ERC1155 Enumerable totalSupply() design is as follows;

 function totalSupply(uint256 id) public view virtual returns (uint256) {
        return _totalSupply(id);
    }

 function _totalSupply(uint256 id) internal view virtual returns (uint256) {
        return ERC1155EnumerableStorage.layout().totalSupply[id];
    }

library ERC1155EnumerableStorage {
    struct Layout {
        mapping(uint256 => uint256) totalSupply;
        mapping(uint256 => EnumerableSet.AddressSet) accountsByToken;
        mapping(address => EnumerableSet.UintSet) tokensByAccount;
    }

    bytes32 internal constant STORAGE_SLOT =
        keccak256('solidstate.contracts.storage.ERC1155Enumerable');

    function layout() internal pure returns (Layout storage l) {
        bytes32 slot = STORAGE_SLOT;
        assembly {
            l.slot := slot
        }
    }
}

Due to faulty design totalSupply() will malfunction and will produce a very important function incorrect figure

In an ERC1155 token contract, the totalSupply() function is used to return the total number of all unique tokens that have been minted for all class of assets in the contract. This function is important because it allows for tracking the total number of NFTs and fungible tokens that have been minted, which can be useful for a variety of purposes such as for tracking the total number of tokens for a particular series, or to ensure that the total number of tokens minted does not exceed a certain limit.

Additionally, this function can be used to check the total supply of a specific class of assets, such as all the gold tokens that have been minted. This can be useful for trading, accounting or other purposes

Recommended Mitigation Steps

Add totalsupply() function in ERC1155 architecture as mentioned above

c4-judge commented 1 year ago

Picodes marked the issue as primary issue

Picodes commented 1 year ago

I may have missed it, but up to my knowledge, the totalSupply version for ERC1155 you are referring is what OpenZeppelin designed but is not part of the EIP. In this case, it'd not be a valid medium finding.

c4-sponsor commented 1 year ago

vhawk19 marked the issue as sponsor disputed

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid