# RACE #5

{% embed url="<https://ventral.digital/posts/2022/3/8/secureum-bootcamp-epoch-march-race-5>" %}
RACE #5
{% endembed %}

<figure><img src="https://3988450783-files.gitbook.io/~/files/v0/b/gitbook-x-prod.appspot.com/o/spaces%2F-MWVjG_njKgBtvmnKaJh%2Fuploads%2F1eikdeY4EEdDshQcghKv%2Fimage.png?alt=media&#x26;token=dd7f361d-7e11-4eb7-8999-ec4312105bc4" alt=""><figcaption><p>RACE #5 result</p></figcaption></figure>

{% hint style="info" %}
\[**Note:** All 8 questions in this quiz are based on the *InSecureum* contract. This is the same contract you will see for all the 8 questions in this quiz. *InSecureum* is adapted from a well-known contract.]
{% endhint %}

```solidity
pragma solidity ^0.8.0;

import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC1155/IERC1155.sol";
import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC1155//IERC1155Receiver.sol";
import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC1155//extensions/IERC1155MetadataURI.sol";
import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/Address.sol";
import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/Context.sol";
import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/introspection/ERC165.sol";

contract InSecureum is Context, ERC165, IERC1155, IERC1155MetadataURI {

    mapping(uint256 => mapping(address => uint256)) private _balances;
    mapping(address => mapping(address => bool)) private _operatorApprovals;
    string private _uri;

    constructor(string memory uri_) {
        _setURI(uri_);
    }

    function supportsInterface(bytes4 interfaceId) public view virtual override(ERC165, IERC165) returns (bool) {
        return
            interfaceId == type(IERC1155).interfaceId ||
            interfaceId == type(IERC1155MetadataURI).interfaceId ||
            super.supportsInterface(interfaceId);
    }

    function uri(uint256) public view virtual override returns (string memory) {
        return _uri;
    }

    function balanceOf(address account, uint256 id) public view virtual override returns (uint256) {
        require(account != address(0), "ERC1155: balance query for the zero address");
        return _balances[id][account];
    }

    function balanceOfBatch(address[] memory accounts, uint256[] memory ids)
        public
        view
        virtual
        override
        returns (uint256[] memory)
    {
        uint256[] memory batchBalances = new uint256[](accounts.length);
        for (uint256 i = 0; i < accounts.length; ++i) {
            batchBalances[i] = balanceOf(accounts[i], ids[i]);
        }
        return batchBalances;
    }

    function setApprovalForAll(address operator, bool approved) public virtual override {
        _setApprovalForAll(_msgSender(), operator, approved);
    }

    function isApprovedForAll(address account, address operator) public view virtual override returns (bool) {
        return _operatorApprovals[account][operator];
    }

    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 owner nor approved"
        );
        _safeTransferFrom(from, to, id, amount, data);
    }

    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: transfer caller is not owner nor approved"
        );
        _safeBatchTransferFrom(from, to, ids, amounts, data);
    }

    function _safeTransferFrom(
        address from,
        address to,
        uint256 id,
        uint256 amount,
        bytes memory data
    ) public virtual {
        address operator = _msgSender();
        uint256 fromBalance = _balances[id][from];
        unchecked {
            fromBalance = fromBalance - amount;
        }
        _balances[id][from] = fromBalance;
        _balances[id][to] += amount;

        emit TransferSingle(operator, from, to, id, amount);
        _doSafeTransferAcceptanceCheck(operator, from, to, id, amount, data);
    }

    function _safeBatchTransferFrom(
        address from,
        address to,
        uint256[] memory ids,
        uint256[] memory amounts,
        bytes memory data
    ) internal virtual {
        require(to != address(0), "ERC1155: transfer to the zero address");
        address operator = _msgSender();
        for (uint256 i = 0; i < ids.length; ++i) {
            uint256 id = ids[i];
            uint256 amount = amounts[i];
            uint256 fromBalance = _balances[id][from];
            fromBalance = fromBalance - amount;
            _balances[id][to] += amount;
        }
        emit TransferBatch(operator, from, to, ids, amounts);
        _doSafeBatchTransferAcceptanceCheck(operator, from, to, ids, amounts, data);
    }

    function _setURI(string memory newuri) internal virtual {
        _uri = newuri;
    }

    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();
        _balances[id][to] += amount;
        emit TransferSingle(operator, address(0), to, id, amount);
        _doSafeTransferAcceptanceCheck(operator, address(0), to, id, amount, data);
    }

    function _mintBatch(
        address to,
        uint256[] memory ids,
        uint256[] memory amounts,
        bytes memory data
    ) internal virtual {
        address operator = _msgSender();
        require(operator != address(0), "ERC1155: mint from the zero address");
        for (uint256 i = 0; i < ids.length; i++) {
            _balances[ids[i]][to] += amounts[i];
        }
        emit TransferBatch(operator, address(0), to, amounts, ids);
        _doSafeBatchTransferAcceptanceCheck(operator, address(0), to, ids, amounts, data);
    }

    function _burn(
        address from,
        uint256 id,
        uint256 amount
    ) internal virtual {
        require(from != address(0), "ERC1155: burn from the zero address");
        address operator = _msgSender();
        uint256 fromBalance = _balances[id][from];
        _balances[id][from] = fromBalance - amount;
        emit TransferSingle(operator, from, address(0), id, amount);
    }

    function _burnBatch(
        address from,
        uint256[] memory ids,
        uint256[] memory amounts
    ) internal virtual {
        require(from != address(0), "ERC1155: burn from the zero address");
        address operator = _msgSender();
        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);
    }

    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);
    }

    function _doSafeTransferAcceptanceCheck(
        address operator,
        address from,
        address to,
        uint256 id,
        uint256 amount,
        bytes memory data
    ) private {
        if (isContract(to)) {
            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 (isContract(to)) {
            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 isContract(address account) internal view returns (bool) {
        return account.code.length == 0;
    }
}
```

## Question 1 :white\_check\_mark:

InSecureum `balanceOf()`

* [ ] &#x20;A. May be optimized by caching state variable in local variable -> The function only contains one `require` and one `return`
* [ ] &#x20;B. May be optimized by changing state mutability from `view` to `pure` -> No, because `_balances` is a state variable
* [ ] &#x20;C. May be optimized by changing its visibility to `external` -> No, because `balanceOfBatch()` calls `balanceOf()`
* [x] &#x20;D. None of the above

**My Comment:**

Here is OpenZeppelin's ERC1155 `balanceOf()` implementation:

```solidity
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];
}
```

## Question 2 :white\_check\_mark:

In InSecureum, array lengths mismatch check is missing in

* [x] A. `balanceOfBatch()`
* [x] &#x20;B. `_safeBatchTransferFrom()`
* [x] &#x20;C. `_mintBatch()`
* [x] &#x20;D. `_burnBatch()`

## Question 3 :white\_check\_mark:

The security concern(s) with InSecureum `_safeTransferFrom()` is/are

* [x] &#x20;A. Incorrect visibility -> Should be `internal`
* [x] &#x20;B. Susceptible to an integer underflow -> Because of `unchecked{}` and no check before that
* [x] &#x20;C. Missing zero-address validation -> `from` and `to` should be checked
* [ ] &#x20;D. None of the above

**My Comment:**

Here is OpenZeppelin's ERC1155 `_safeTransferFrom()` implementation:

```solidity
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);
}
```

## Question 4 :white\_check\_mark:

The security concern(s) with InSecureum `_safeBatchTransferFrom()` is/are

* [x] A. Missing array lengths mismatch check
* [ ] &#x20;B. Susceptibility to an integer underflow
* [x] &#x20;C. Incorrect balance update -> `_balances[id][from]` is stored in a local variable but it is never updated
* [ ] &#x20;D. None of the above

**My Comment:**

Here is OpenZeppelin's ERC1155 `_safeBatchTransferFrom()` implementation:

```solidity
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);
}
```

## Question 5 :white\_check\_mark:

The security concern(s) with InSecureum `_mintBatch()` is/are

* [x] &#x20;A. Missing array lengths mismatch check
* [x] &#x20;B. Incorrect event emission -> Should be `emit TransferBatch(operator, address(0), to, ids, amounts)`
* [x] &#x20;C. Allows burning of tokens -> `to` can be zero address
* [ ] &#x20;D. None of the above

**My Comment:**

Here is OpenZeppelin's ERC1155 `_mintBatch()` implementation:

```solidity
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);
}
```

## Question 6 :white\_check\_mark:

The security concern(s) with InSecureum `_burn()` is/are

* [ ] &#x20;A. Missing zero-address validation
* [ ] &#x20;B. Susceptibility to an integer underflow
* [ ] &#x20;C. Incorrect balance update
* [x] &#x20;D. None of the above

**My Comment:**

Here is OpenZeppelin's ERC1155 `_burn()` implementation:

```solidity
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, "");
}
```

## Question 7 :white\_check\_mark:

The security concern(s) with InSecureum `_doSafeTransferAcceptanceCheck()` is/are

* [ ] &#x20;A. `isContract` check on incorrect address
* [x] &#x20;B. Incorrect check on return value -> Should be `if (response != IERC1155Receiver.onERC1155Received.selector)`
* [x] &#x20;C. Call to incorrect `isContract` implementation
* [ ] &#x20;D. None of the above

**My Comment:**

Here is OpenZeppelin's ERC1155 `_doSafeTransferAcceptanceCheck()` implementation:

```solidity
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");
        }
    }
}
```

## Question 8 :white\_check\_mark:

The security concern(s) with InSecureum `isContract()` implementation is/are

* [ ] &#x20;A. Incorrect visibility
* [x] &#x20;B. Incorrect operator in the comparison -> Should be `return account.code.length != 0`
* [ ] &#x20;C. Unnecessary because Ethereum only has Contract accounts
* [ ] &#x20;D. None of the above