# RACE #7 - Bored Ape

{% embed url="<https://ventral.digital/posts/2022/07/secureum-bootcamp-epoch-june-race-7>" %}
RACE #7
{% endembed %}

<figure><img src="https://3988450783-files.gitbook.io/~/files/v0/b/gitbook-x-prod.appspot.com/o/spaces%2F-MWVjG_njKgBtvmnKaJh%2Fuploads%2FdYiOYNOP6hLBOqhqkEJG%2Fimage.png?alt=media&#x26;token=22e4d52f-6fb8-40a0-8e77-b64328294fa4" alt=""><figcaption><p>RACE #7 result</p></figcaption></figure>

{% hint style="info" %}
*Note: All 8 questions in this RACE are based on the InSecureumApe contract. This is the same contract you will see for all the 8 questions in this RACE. InSecureumApe is adapted from a well-known contract. The question is below the shown contract.*
{% endhint %}

```solidity
pragma solidity ^0.7.0;

import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/release-v3.4/contracts/access/Ownable.sol";
import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/release-v3.4/contracts/token/ERC721/ERC721.sol";
import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/release-v3.4/contracts/math/SafeMath.sol";

contract InSecureumApe is ERC721, Ownable {
    using SafeMath for uint256;

    string public IA_PROVENANCE = "";

    uint256 public startingIndexBlock;

    uint256 public startingIndex;

    uint256 public constant apePrice = 800000000000000000; //0.08 ETH

    uint public constant maxApePurchase = 20;

    uint256 public MAX_APES;

    bool public saleIsActive = false;

    uint256 public REVEAL_TIMESTAMP;

    constructor(string memory name, string memory symbol, uint256 maxNftSupply, uint256 saleStart) ERC721(name, symbol) {
        MAX_APES = maxNftSupply;
        REVEAL_TIMESTAMP = saleStart + (86400 * 9);
    }

    function withdraw() public onlyOwner {
        uint balance = address(this).balance;
        msg.sender.transfer(balance);
    }

    function reserveApes() public onlyOwner {
        uint supply = totalSupply();
        uint i;
        for (i = 0; i < 30; i++) {
            _safeMint(msg.sender, supply + i);
        }
    }

    function setRevealTimestamp(uint256 revealTimeStamp) public onlyOwner {
        REVEAL_TIMESTAMP = revealTimeStamp;
    }

    function setProvenanceHash(string memory provenanceHash) public onlyOwner {
        IA_PROVENANCE = provenanceHash;
    }

    function setBaseURI(string memory baseURI) public onlyOwner {
        _setBaseURI(baseURI);
    }

    function flipSaleState() public onlyOwner {
        saleIsActive = !saleIsActive;
    }

    function mintApe(uint numberOfTokens) public payable {
        require(saleIsActive, "Sale must be active to mint Ape");
        require(numberOfTokens < maxApePurchase, "Can only mint 20 tokens at a time");
        require(totalSupply().add(numberOfTokens) <= MAX_APES, "Purchase would exceed max supply of Apes");
        require(apePrice.mul(numberOfTokens) <= msg.value, "Ether value sent is not correct");

        for(uint i = 0; i < numberOfTokens; i++) {
            uint mintIndex = totalSupply();
            if (totalSupply() < MAX_APES) {
                _safeMint(msg.sender, mintIndex);
            }
        }

        // If we haven't set the starting index and this is either 1) the last saleable token or 2) the first token to be sold after
        // the end of pre-sale, set the starting index block
        if (startingIndexBlock == 0 && (totalSupply() == MAX_APES || block.timestamp >= REVEAL_TIMESTAMP)) {
            startingIndexBlock = block.number;
        }
    }

    function setStartingIndex() public {
        require(startingIndex == 0, "Starting index is already set");
        require(startingIndexBlock != 0, "Starting index block must be set");

        startingIndex = uint(blockhash(startingIndexBlock)) % MAX_APES;
        if (block.number.sub(startingIndexBlock) > 255) {
            startingIndex = uint(blockhash(block.number - 1)) % MAX_APES;
        }
        if (startingIndex == 0) {
            startingIndex = startingIndex.add(1);
        }
    }

    function emergencySetStartingIndexBlock() public onlyOwner {
        require(startingIndex == 0, "Starting index is already set");
        startingIndexBlock = block.number;
    }
}
```

## Question 1 :white\_check\_mark:

The mint price of an InSecureumApe is:

* [ ] &#x20;A. 0.0008 ETH
* [ ] &#x20;B. 0.008 ETH
* [ ] &#x20;C. 0.08 ETH
* [x] &#x20;D. 0.8 ETH -> The inline comment is wrong, do the computation on your own

**My comment:**

<figure><img src="https://3988450783-files.gitbook.io/~/files/v0/b/gitbook-x-prod.appspot.com/o/spaces%2F-MWVjG_njKgBtvmnKaJh%2Fuploads%2FcWAHv0ZQUg8wA2YN2j3K%2Fimage.png?alt=media&#x26;token=53d89c97-072d-4727-9d5a-e460e7bd1b5f" alt=""><figcaption><p> </p></figcaption></figure>

## Question 2 :white\_check\_mark:

The security concern(s) with InSecureumApe access control is/are

* [x] &#x20;A. Owner can arbitrarily pause public minting of InSecureumApes -> function `flipSaleState()`
* [x] &#x20;B. Owner can arbitrarily mint InSecureumApes -> function `reserveApes()`
* [x] &#x20;C. Single-step ownership change -> `transferOwnership()` from OpenZeppelin `Ownable.sol`
* [x] &#x20;D. Missing event emits in and time-delayed effects of owner functions -> None of the owner functions emits event or having time-delayed feature

## Question 3 :white\_check\_mark:

The security concern(s) with InSecureumApe constructor is/are

* [x] &#x20;A. Missing sanity/threshold check on maxNftSupply -> No `require` statement for it
* [x] &#x20;B. Missing sanity/threshold check on saleStart -> No `require` statement for it
* [x] &#x20;C. Potential integer overflow -> Compiler is Solidity v0.7, `REVEAL_TIMESTAMP = saleStart + (86400 * 9)` could overflow
* [ ] &#x20;D. None of the above

## Question 4 :white\_check\_mark:

The total number of InSecureumApes that can ever be minted is

* [ ] &#x20;A. maxApePurchase
* [ ] &#x20;B. MAX\_APES
* [ ] &#x20;C. MAX\_APES + 30
* [x] &#x20;D. type(uint256).max -> `reserveApes()` does not check for `MAX_APES` upperbound

## Question 5 :white\_check\_mark:

The public minting of InSecureumApes

* [ ] &#x20;A. Must be paid the exact amount in Ether -> `require(apePrice.mul(numberOfTokens) <= msg.value`, not `== msg.value`
* [x] &#x20;B. May be performed 19 NFTs at a time -> `require(numberOfTokens < maxApePurchase` and `maxApePurchase == 20`
* [x] &#x20;C. Uses \_safeMint to prevent locked/stuck NFTs -> `_safeMint()` requires the receiver to implement the `onERC721Received()` callback
* [ ] &#x20;D. None of the above

## Question 6 :white\_check\_mark:

The security concerns with InSecureumApe is/are

* [x] &#x20;A. Use of a floating pragma and an older compiler version -> Susceptible to integer overflow/underflow
* [ ] &#x20;B. Oracle price manipulation -> This contract does not use oracle, only uses a constant price `apePrice`
* [x] &#x20;C. Reentrancy allowing bypass of maxApePurchase check -> `_safeMint()` triggers the `onERC721Received()` callback, attacker can call `mintApe(19)` over and over
* [ ] &#x20;D. None of the above

## Question 7 - This one tricky

The starting index determination

* [x] &#x20;A. Is meant to randomize NFT reveal post-mint
* [x] &#x20;B. Can be triggered by the owner at any time -> Function `emergencySetStartingIndexBlock()`
* [ ] &#x20;C. May be triggered only 9 days after sale start -> Can be triggered anytime by `emergencySetStartingIndexBlock()`
* [x] &#x20;D. Accounts for the fact that EVM only stores previous 256 block hashes

**Comment:**

You can read about how this is used for post-mint reveal randomization in [this article](https://medium.com/web-design-web-developer-magazine/the-offset-approach-to-fair-nft-reveals-and-other-metadata-reveal-strategies-considerations-2e2c69e5c274).

The 9-day delay of the `REVEAL_TIMESTAMP` variable can be overriden at any point in time, it can also be triggered earlier if the totalSupply matches `MAX_APES` exactly, or be triggered at any time by the owner via `emergencySetStartingIndexBlock()`.

It accounts for the block hash access limitation by falling back to using the hash of the previous block instead.

## Question 8 :white\_check\_mark:

Potential gas optimization(s) in InSecureumApe is/are

* [x] &#x20;A. Caching of storage variables
* [x] &#x20;B. Avoiding initializations of variables to default values of their types
* [x] &#x20;C. Use of immutables
* [ ] &#x20;D. None of the above

**Comment:**

Whenever storage variables are read from multiple times, they should be cached in memory to safe gas. This is missing for `MAX_APES` in `mintApe()` and `startingIndexBlock` in `setStartingIndex()`.

All state variables are zero-initialized by default, therefore there's no need to manually set `saleIsActive` to false, for example.

The state variable `MAX_APES` is only set once during construction and should be immutable to save gas