diff --git a/audits/202309-threat-model-preset-erc721.md b/audits/202309-threat-model-preset-erc721.md index 731bc47a..2452237f 100644 --- a/audits/202309-threat-model-preset-erc721.md +++ b/audits/202309-threat-model-preset-erc721.md @@ -1,5 +1,3 @@ -# Contract Factory Threat Model - ## Introduction This document is a thread model for two preset erc721 token contracts built by Immutable. diff --git a/audits/202312-threat-model-preset-erc1155.md b/audits/202312-threat-model-preset-erc1155.md new file mode 100644 index 00000000..35c8e978 --- /dev/null +++ b/audits/202312-threat-model-preset-erc1155.md @@ -0,0 +1,42 @@ +## Introduction +This document is a thread model for the preset erc1155 token contracts built by Immutable. + +This document encompasses information for all contracts under the [token](../contracts/token/erc1155) directory + +## Context + +The ERC1155 presets built by immutable were done with the requirements of supply tracking and permits + +- Clients should be able to track how many tokens of a specific token id in a collection is in circulation + +- Clients should be able to create permits for unapproved wallets to operate on their behalf + +- Minting should be restricted to addresses that were granted the `minter` role. + +- Only allow operators should be able to modify and assign roles to addresses for administering the collection on chain. + +- Contracts should not be upgradeable to prevent external developers from getting around royalty requirements. + + +## Design and Implementation + +### ImmutableERC1155 +The ImmutableERC1155 extends OZ's `ERC1155Burnable` contract inheriting the public burn methods to be used by the client. +Permit is added to allow for Gasless transactions from the token owners. + +#### Modifications From Base Implementation + +- Added total supply tracking for each token id. This will be managed via the pre-transfer hook called by mint, burn and transfer methods +- Added Permits to allow unapproved wallets to become approved without the owner spending gas. +- Override `uri` to return `baseURI` field to keep in standard with ImmutableERC721 +- Added `baseURI` to replace `uri` to encourage the usage of `baseURI` + + +## Attack Surfaces + +ERC1155 only has `setApproveForAll` as it's approval method. Meaning any flow that requires a 3rd party to operator on a set of tokens owned by another wallet will grant the third party access to all of that specific wallet's tokens. The third party needs to be entirely trustworthy. The owner needs to be diligent on revoking unrestricted access when not needed. + +We can consider implementing a more complicated approval schema if needed. i.e by token id or by token id and amount. + +## Tests +`forge test` will run all the related tests. \ No newline at end of file diff --git a/contracts/token/erc1155/abstract/ERC1155.sol b/contracts/token/erc1155/abstract/ERC1155.sol deleted file mode 100644 index 8bea4c1f..00000000 --- a/contracts/token/erc1155/abstract/ERC1155.sol +++ /dev/null @@ -1,497 +0,0 @@ -// SPDX-License-Identifier: MIT -// OpenZeppelin Contracts (last updated v4.9.0) (token/ERC1155/ERC1155.sol) - -pragma solidity 0.8.19; - -import {IERC1155} from "@openzeppelin/contracts/token/ERC1155/IERC1155.sol"; -import {IERC1155Receiver} from "@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol"; -import {IERC1155MetadataURI} from "@openzeppelin/contracts/token/ERC1155/extensions/IERC1155MetadataURI.sol"; -import {Context} from "@openzeppelin/contracts/utils/Context.sol"; -import {IERC165, ERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; -import "@openzeppelin/contracts/utils/Address.sol"; - -/** - * @dev Implementation of the basic standard multi-token. - * See https://eips.ethereum.org/EIPS/eip-1155 - * Originally based on code by Enjin: https://github.com/enjin/erc-1155 - * - * _Available since v3.1._ - */ -contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { - using Address for address; - - // Mapping from token ID to account balances - mapping(uint256 => mapping(address => uint256)) private _balances; - - // Mapping from account to operator approvals - mapping(address => mapping(address => bool)) private _operatorApprovals; - - // Used as the URI for all token types by relying on ID substitution, e.g. https://token-cdn-domain/{id}.json - string private _uri; - - /** - * @dev See {_setURI}. - */ - constructor(string memory uri_) { - _setURI(uri_); - } - - /** - * @dev See {IERC165-supportsInterface}. - */ - function supportsInterface(bytes4 interfaceId) public view virtual override(ERC165, IERC165) returns (bool) { - return - interfaceId == type(IERC1155).interfaceId || - interfaceId == type(IERC1155MetadataURI).interfaceId || - super.supportsInterface(interfaceId); - } - - /** - * @dev See {IERC1155MetadataURI-uri}. - * - * This implementation returns the same URI for *all* token types. It relies - * on the token type ID substitution mechanism - * https://eips.ethereum.org/EIPS/eip-1155#metadata[defined in the EIP]. - * - * Clients calling this function must replace the `\{id\}` substring with the - * actual token type ID. - */ - function uri(uint256) public view virtual override returns (string memory) { - return _uri; - } - - /** - * @dev See {IERC1155-balanceOf}. - * - * Requirements: - * - * - `account` cannot be the zero address. - */ - function balanceOf(address account, uint256 id) public view virtual override returns (uint256) { - require(account != address(0), "ERC1155: address zero is not a valid owner"); - return _balances[id][account]; - } - - /** - * @dev See {IERC1155-balanceOfBatch}. - * - * Requirements: - * - * - `accounts` and `ids` must have the same length. - */ - function balanceOfBatch( - address[] memory accounts, - uint256[] memory ids - ) public view virtual override returns (uint256[] memory) { - require(accounts.length == ids.length, "ERC1155: accounts and ids length mismatch"); - - uint256[] memory batchBalances = new uint256[](accounts.length); - - for (uint256 i = 0; i < accounts.length; ++i) { - batchBalances[i] = balanceOf(accounts[i], ids[i]); - } - - return batchBalances; - } - - /** - * @dev See {IERC1155-setApprovalForAll}. - */ - function setApprovalForAll(address operator, bool approved) public virtual override { - _setApprovalForAll(_msgSender(), operator, approved); - } - - /** - * @dev See {IERC1155-isApprovedForAll}. - */ - function isApprovedForAll(address account, address operator) public view virtual override returns (bool) { - return _operatorApprovals[account][operator]; - } - - /** - * @dev See {IERC1155-safeTransferFrom}. - */ - function safeTransferFrom( - address from, - address to, - uint256 id, - uint256 amount, - bytes memory data - ) public virtual override { - require( - from == _msgSender() || isApprovedForAll(from, _msgSender()), - "ERC1155: caller is not token owner or approved" - ); - _safeTransferFrom(from, to, id, amount, data); - } - - /** - * @dev See {IERC1155-safeBatchTransferFrom}. - */ - function safeBatchTransferFrom( - address from, - address to, - uint256[] memory ids, - uint256[] memory amounts, - bytes memory data - ) public virtual override { - require( - from == _msgSender() || isApprovedForAll(from, _msgSender()), - "ERC1155: caller is not token owner or approved" - ); - _safeBatchTransferFrom(from, to, ids, amounts, data); - } - - /** - * @dev Transfers `amount` tokens of token type `id` from `from` to `to`. - * - * Emits a {TransferSingle} event. - * - * Requirements: - * - * - `to` cannot be the zero address. - * - `from` must have a balance of tokens of type `id` of at least `amount`. - * - If `to` refers to a smart contract, it must implement {IERC1155Receiver-onERC1155Received} and return the - * acceptance magic value. - */ - function _safeTransferFrom( - address from, - address to, - uint256 id, - uint256 amount, - bytes memory data - ) internal virtual { - require(to != address(0), "ERC1155: transfer to the zero address"); - - address operator = _msgSender(); - uint256[] memory ids = _asSingletonArray(id); - uint256[] memory amounts = _asSingletonArray(amount); - - _beforeTokenTransfer(operator, from, to, ids, amounts, data); - - uint256 fromBalance = _balances[id][from]; - require(fromBalance >= amount, "ERC1155: insufficient balance for transfer"); - unchecked { - _balances[id][from] = fromBalance - amount; - } - _balances[id][to] += amount; - - emit TransferSingle(operator, from, to, id, amount); - - _afterTokenTransfer(operator, from, to, ids, amounts, data); - - _doSafeTransferAcceptanceCheck(operator, from, to, id, amount, data); - } - - /** - * @dev xref:ROOT:erc1155.adoc#batch-operations[Batched] version of {_safeTransferFrom}. - * - * Emits a {TransferBatch} event. - * - * Requirements: - * - * - If `to` refers to a smart contract, it must implement {IERC1155Receiver-onERC1155BatchReceived} and return the - * acceptance magic value. - */ - function _safeBatchTransferFrom( - address from, - address to, - uint256[] memory ids, - uint256[] memory amounts, - bytes memory data - ) internal virtual { - require(ids.length == amounts.length, "ERC1155: ids and amounts length mismatch"); - require(to != address(0), "ERC1155: transfer to the zero address"); - - address operator = _msgSender(); - - _beforeTokenTransfer(operator, from, to, ids, amounts, data); - - for (uint256 i = 0; i < ids.length; ++i) { - uint256 id = ids[i]; - uint256 amount = amounts[i]; - - uint256 fromBalance = _balances[id][from]; - require(fromBalance >= amount, "ERC1155: insufficient balance for transfer"); - unchecked { - _balances[id][from] = fromBalance - amount; - } - _balances[id][to] += amount; - } - - emit TransferBatch(operator, from, to, ids, amounts); - - _afterTokenTransfer(operator, from, to, ids, amounts, data); - - _doSafeBatchTransferAcceptanceCheck(operator, from, to, ids, amounts, data); - } - - /** - * @dev Sets a new URI for all token types, by relying on the token type ID - * substitution mechanism - * https://eips.ethereum.org/EIPS/eip-1155#metadata[defined in the EIP]. - * - * By this mechanism, any occurrence of the `\{id\}` substring in either the - * URI or any of the amounts in the JSON file at said URI will be replaced by - * clients with the token type ID. - * - * For example, the `https://token-cdn-domain/\{id\}.json` URI would be - * interpreted by clients as - * `https://token-cdn-domain/000000000000000000000000000000000000000000000000000000000004cce0.json` - * for token type ID 0x4cce0. - * - * See {uri}. - * - * Because these URIs cannot be meaningfully represented by the {URI} event, - * this function emits no events. - */ - function _setURI(string memory newuri) internal virtual { - _uri = newuri; - } - - /** - * @dev Creates `amount` tokens of token type `id`, and assigns them to `to`. - * - * Emits a {TransferSingle} event. - * - * Requirements: - * - * - `to` cannot be the zero address. - * - If `to` refers to a smart contract, it must implement {IERC1155Receiver-onERC1155Received} and return the - * acceptance magic value. - */ - function _mint(address to, uint256 id, uint256 amount, bytes memory data) internal virtual { - require(to != address(0), "ERC1155: mint to the zero address"); - - address operator = _msgSender(); - uint256[] memory ids = _asSingletonArray(id); - uint256[] memory amounts = _asSingletonArray(amount); - - _beforeTokenTransfer(operator, address(0), to, ids, amounts, data); - - _balances[id][to] += amount; - emit TransferSingle(operator, address(0), to, id, amount); - - _afterTokenTransfer(operator, address(0), to, ids, amounts, data); - - _doSafeTransferAcceptanceCheck(operator, address(0), to, id, amount, data); - } - - /** - * @dev xref:ROOT:erc1155.adoc#batch-operations[Batched] version of {_mint}. - * - * Emits a {TransferBatch} event. - * - * Requirements: - * - * - `ids` and `amounts` must have the same length. - * - If `to` refers to a smart contract, it must implement {IERC1155Receiver-onERC1155BatchReceived} and return the - * acceptance magic value. - */ - function _mintBatch( - address to, - uint256[] memory ids, - uint256[] memory amounts, - bytes memory data - ) internal virtual { - require(to != address(0), "ERC1155: mint to the zero address"); - require(ids.length == amounts.length, "ERC1155: ids and amounts length mismatch"); - - address operator = _msgSender(); - - _beforeTokenTransfer(operator, address(0), to, ids, amounts, data); - - for (uint256 i = 0; i < ids.length; i++) { - _balances[ids[i]][to] += amounts[i]; - } - - emit TransferBatch(operator, address(0), to, ids, amounts); - - _afterTokenTransfer(operator, address(0), to, ids, amounts, data); - - _doSafeBatchTransferAcceptanceCheck(operator, address(0), to, ids, amounts, data); - } - - /** - * @dev Destroys `amount` tokens of token type `id` from `from` - * - * Emits a {TransferSingle} event. - * - * Requirements: - * - * - `from` cannot be the zero address. - * - `from` must have at least `amount` tokens of token type `id`. - */ - function _burn(address from, uint256 id, uint256 amount) internal virtual { - require(from != address(0), "ERC1155: burn from the zero address"); - - address operator = _msgSender(); - uint256[] memory ids = _asSingletonArray(id); - uint256[] memory amounts = _asSingletonArray(amount); - - _beforeTokenTransfer(operator, from, address(0), ids, amounts, ""); - - uint256 fromBalance = _balances[id][from]; - require(fromBalance >= amount, "ERC1155: burn amount exceeds balance"); - unchecked { - _balances[id][from] = fromBalance - amount; - } - - emit TransferSingle(operator, from, address(0), id, amount); - - _afterTokenTransfer(operator, from, address(0), ids, amounts, ""); - } - - /** - * @dev xref:ROOT:erc1155.adoc#batch-operations[Batched] version of {_burn}. - * - * Emits a {TransferBatch} event. - * - * Requirements: - * - * - `ids` and `amounts` must have the same length. - */ - function _burnBatch(address from, uint256[] memory ids, uint256[] memory amounts) internal virtual { - require(from != address(0), "ERC1155: burn from the zero address"); - require(ids.length == amounts.length, "ERC1155: ids and amounts length mismatch"); - - address operator = _msgSender(); - - _beforeTokenTransfer(operator, from, address(0), ids, amounts, ""); - - for (uint256 i = 0; i < ids.length; i++) { - uint256 id = ids[i]; - uint256 amount = amounts[i]; - - uint256 fromBalance = _balances[id][from]; - require(fromBalance >= amount, "ERC1155: burn amount exceeds balance"); - unchecked { - _balances[id][from] = fromBalance - amount; - } - } - - emit TransferBatch(operator, from, address(0), ids, amounts); - - _afterTokenTransfer(operator, from, address(0), ids, amounts, ""); - } - - /** - * @dev Approve `operator` to operate on all of `owner` tokens - * - * Emits an {ApprovalForAll} event. - */ - function _setApprovalForAll(address owner, address operator, bool approved) internal virtual { - require(owner != operator, "ERC1155: setting approval status for self"); - _operatorApprovals[owner][operator] = approved; - emit ApprovalForAll(owner, operator, approved); - } - - /** - * @dev Hook that is called before any token transfer. This includes minting - * and burning, as well as batched variants. - * - * The same hook is called on both single and batched variants. For single - * transfers, the length of the `ids` and `amounts` arrays will be 1. - * - * Calling conditions (for each `id` and `amount` pair): - * - * - When `from` and `to` are both non-zero, `amount` of ``from``'s tokens - * of token type `id` will be transferred to `to`. - * - When `from` is zero, `amount` tokens of token type `id` will be minted - * for `to`. - * - when `to` is zero, `amount` of ``from``'s tokens of token type `id` - * will be burned. - * - `from` and `to` are never both zero. - * - `ids` and `amounts` have the same, non-zero length. - * - * To learn more about hooks, head to xref:ROOT:extending-contracts.adoc#using-hooks[Using Hooks]. - */ - function _beforeTokenTransfer( - address operator, - address from, - address to, - uint256[] memory ids, - uint256[] memory amounts, - bytes memory data - ) internal virtual {} - - /** - * @dev Hook that is called after any token transfer. This includes minting - * and burning, as well as batched variants. - * - * The same hook is called on both single and batched variants. For single - * transfers, the length of the `id` and `amount` arrays will be 1. - * - * Calling conditions (for each `id` and `amount` pair): - * - * - When `from` and `to` are both non-zero, `amount` of ``from``'s tokens - * of token type `id` will be transferred to `to`. - * - When `from` is zero, `amount` tokens of token type `id` will be minted - * for `to`. - * - when `to` is zero, `amount` of ``from``'s tokens of token type `id` - * will be burned. - * - `from` and `to` are never both zero. - * - `ids` and `amounts` have the same, non-zero length. - * - * To learn more about hooks, head to xref:ROOT:extending-contracts.adoc#using-hooks[Using Hooks]. - */ - function _afterTokenTransfer( - address operator, - address from, - address to, - uint256[] memory ids, - uint256[] memory amounts, - bytes memory data - ) internal virtual {} - - function _doSafeTransferAcceptanceCheck( - address operator, - address from, - address to, - uint256 id, - uint256 amount, - bytes memory data - ) private { - if (to.isContract()) { - try IERC1155Receiver(to).onERC1155Received(operator, from, id, amount, data) returns (bytes4 response) { - if (response != IERC1155Receiver.onERC1155Received.selector) { - revert("ERC1155: ERC1155Receiver rejected tokens"); - } - } catch Error(string memory reason) { - revert(reason); - } catch { - revert("ERC1155: transfer to non-ERC1155Receiver implementer"); - } - } - } - - function _doSafeBatchTransferAcceptanceCheck( - address operator, - address from, - address to, - uint256[] memory ids, - uint256[] memory amounts, - bytes memory data - ) private { - if (to.isContract()) { - try IERC1155Receiver(to).onERC1155BatchReceived(operator, from, ids, amounts, data) returns ( - bytes4 response - ) { - if (response != IERC1155Receiver.onERC1155BatchReceived.selector) { - revert("ERC1155: ERC1155Receiver rejected tokens"); - } - } catch Error(string memory reason) { - revert(reason); - } catch { - revert("ERC1155: transfer to non-ERC1155Receiver implementer"); - } - } - } - - function _asSingletonArray(uint256 element) private pure returns (uint256[] memory) { - uint256[] memory array = new uint256[](1); - array[0] = element; - - return array; - } -} \ No newline at end of file diff --git a/contracts/token/erc1155/abstract/ERC1155Burnable.sol b/contracts/token/erc1155/abstract/ERC1155Burnable.sol deleted file mode 100644 index 4ca8b2cd..00000000 --- a/contracts/token/erc1155/abstract/ERC1155Burnable.sol +++ /dev/null @@ -1,32 +0,0 @@ -// SPDX-License-Identifier: MIT -// OpenZeppelin Contracts (last updated v4.9.0) (token/ERC1155/extensions/ERC1155Burnable.sol) - -pragma solidity 0.8.19; - -import "./ERC1155.sol"; - -/** - * @dev Extension of {ERC1155} that allows token holders to destroy both their - * own tokens and those that they have been approved to use. - * - * _Available since v3.1._ - */ -abstract contract ERC1155Burnable is ERC1155 { - function burn(address account, uint256 id, uint256 value) public virtual { - require( - account == _msgSender() || isApprovedForAll(account, _msgSender()), - "ERC1155: caller is not token owner or approved" - ); - - _burn(account, id, value); - } - - function burnBatch(address account, uint256[] memory ids, uint256[] memory values) public virtual { - require( - account == _msgSender() || isApprovedForAll(account, _msgSender()), - "ERC1155: caller is not token owner or approved" - ); - - _burnBatch(account, ids, values); - } -} \ No newline at end of file diff --git a/contracts/token/erc1155/abstract/ERC1155Permit.Sol b/contracts/token/erc1155/abstract/ERC1155Permit.Sol index 7afcc3e0..5f185d92 100644 --- a/contracts/token/erc1155/abstract/ERC1155Permit.Sol +++ b/contracts/token/erc1155/abstract/ERC1155Permit.Sol @@ -1,7 +1,7 @@ pragma solidity 0.8.19; // SPDX-License-Identifier: MIT -import "./ERC1155Burnable.sol"; +import "@openzeppelin/contracts/token/ERC1155/extensions/ERC1155Burnable.sol"; import "@openzeppelin/contracts/utils/cryptography/EIP712.sol"; import "@openzeppelin/contracts/interfaces/IERC1271.sol"; import "solidity-bytes-utils/contracts/BytesLib.sol";