Quiz

Secureum Bootcamp Audit Findings 201 Quiz • Ventral Digitalventraldigital

Slot 8 Quiz

Note: All 8 questions in this quiz are based on the InSecureumNFT contract snippet. This is the same contract snippet you will see for all the 8 questions in this quiz.

InSecureumNFT is a NFT project that aims to distribute CryptoSAFU NFTs to its community where most of them are fairdropped based on past contributions and a few are sold.

CryptoSAFUs with lower IDs have more unique traits, may be valued higher and therefore require a random distribution for fairness.

Assume that all strictly required ERC721 functionality (not shown) and any other required functionality (not shown) are implemented correctly.

Only functionality specific to the sale and minting of NFTs is shown in this contract snippet.

pragma solidity 0.8.0;

interface ERC721TokenReceiver{function onERC721Received(address _operator, address _from, uint256 _tokenId, bytes calldata _data) external returns(bytes4);}

// Assume that all strictly required ERC721 functionality (not shown) is implemented correctly
// Assume that any other required functionality (not shown) is implemented correctly
contract InSecureumNFT {
    bytes4 internal constant MAGIC_ERC721_RECEIVED = 0x150b7a02;
    uint public constant TOKEN_LIMIT = 10; // 10 for testing, 13337 for production
    uint public constant SALE_LIMIT = 5; // 5 for testing, 1337 for production

    mapping (uint256 => address) internal idToOwner;
    uint internal numTokens = 0;
    uint internal numSales = 0;
    address payable internal deployer;
    address payable internal beneficiary;
    bool public publicSale = false;
    uint private price;
    uint public saleStartTime;
    uint public constant saleDuration = 13*13337; // 13337 blocks assuming 13s block times 
    uint internal nonce = 0;
    uint[TOKEN_LIMIT] internal indices;
 
    constructor(address payable _beneficiary) {
        deployer = payable(msg.sender);
        beneficiary = _beneficiary;
    }

    function startSale(uint _price) external {
        require(msg.sender == deployer || _price != 0, "Only deployer and price cannot be zero");
        price = _price;
        saleStartTime = block.timestamp;
        publicSale = true;
    }

    function isContract(address _addr) internal view returns (bool addressCheck) {
        uint256 size;
        assembly { size := extcodesize(_addr) }
        addressCheck = size > 0;
    }

    function randomIndex() internal returns (uint) {
        uint totalSize = TOKEN_LIMIT - numTokens;
        uint index = uint(keccak256(abi.encodePacked(nonce, msg.sender, block.difficulty, block.timestamp))) % totalSize;
        uint value = 0;
        if (indices[index] != 0) {
            value = indices[index];
        } else {
            value = index;
        }
        if (indices[totalSize - 1] == 0) {
            indices[index] = totalSize - 1;
        } else {
            indices[index] = indices[totalSize - 1];
        }
        nonce += 1;
        return (value + 1);
    }

    // Calculate the mint price
    function getPrice() public view returns (uint) {
        require(publicSale, "Sale not started.");
        uint elapsed = block.timestamp - saleStartTime;
        if (elapsed > saleDuration) {
            return 0;
        } else {
            return ((saleDuration - elapsed) * price) / saleDuration;
        }
    }
    
    // SALE_LIMIT is 1337 
    // Rest i.e. (TOKEN_LIMIT - SALE_LIMIT) are reserved for community distribution (not shown)
    function mint() external payable returns (uint) {
        require(publicSale, "Sale not started.");
        require(numSales < SALE_LIMIT, "Sale limit reached.");
        numSales++;
        uint salePrice = getPrice();
        require((address(this)).balance >= salePrice, "Insufficient funds to purchase.");
        if ((address(this)).balance >= salePrice) {
            payable(msg.sender).transfer((address(this)).balance - salePrice);
        }
        return _mint(msg.sender);
    }

    // TOKEN_LIMIT is 13337
    function _mint(address _to) internal returns (uint) {
        require(numTokens < TOKEN_LIMIT, "Token limit reached.");
        // Lower indexed/numbered NFTs have rare traits and may be considered
        // as more valuable by buyers => Therefore randomize
        uint id = randomIndex();
        if (isContract(_to)) {
            bytes4 retval = ERC721TokenReceiver(_to).onERC721Received(msg.sender, address(0), id, "");
            require(retval == MAGIC_ERC721_RECEIVED);
        }
        require(idToOwner[id] == address(0), "Cannot add, already owned.");
        idToOwner[id] = _to;
        numTokens = numTokens + 1;
        beneficiary.transfer((address(this)).balance);
        return id;
    }
}

---

Q1 Missing zero-address check(s) in the contract ✅

• A) May allow anyone to start the sale -> This is true but has nothing to do with zero address check

• B) May put the NFT sale proceeds at risk -> _beneficiary shouldn't be the zero address, otherwise the NFT sale proceeds will be sent to the zero address

• C) May burn the newly minted NFTs -> _mint(msg.sender) is called in mint(), the parameter of _mint() is not controlled by users, all good

• D) None of the above

Comment:

While the require statement in startSale() states that only the deployer may call the function AND the price needs to be not zero, the actual code uses OR which allows anyone to start the sale as long as they specify a valid price - but that can't be fixed by adding a zero-address check. All proceeds appear to be intended to go to the benificiary and since there's no validation of the _benificiary address when it is set during construction, a zero-address could indeed put the sale proceeds to risk. In the given code, the internal _mint(_to) function is always called with msg.sender as _to value which can't be a zero-address.

Q2 Given that lower indexed/numbered CryptoSAFU NFTs have rarer traits (and are considered more valuable as commented in _mint), the implementation of InSecureumNFT is susceptible to the following exploits

• A) Buyers can repeatedly mint and revert until they receive desired NFT -> nonce will change

• B) Buyers can generate addresses to mint until they receive desired NFT -> msg.sender will chagne

• C) Miners can manipulate block.timestamp to facilitate minting of desired NFT

• D) None of the above

Comment:

The index of a CryptoSAFU NFT depends on a nonce that increases after every mint and has an internal visibility preventing contracts to read its current value easily, which would allow them to predict an index for the current block. But a prediction is not necessary since a contract can simply call _mint() repeatedly every block and revert if the result is not desired, ensuring a refund. The msg.sender is indeed also a variable for the "random" index generation, although it's very effective exploiting it, since you'd still have to pay the full price for each of those attempts because the nonce will change after each buy. There's also no need to generate a new address, you can just keep buying using the same address until you receive the desired NFT. A miner would indeed be able to pre-calculate a desirable index off-chain by picking a specific block.timestamp and adding their mint-transaction to the beginning of their block.

Q3 The getPrice() function

• A) Is expected to reduce the mint price over time after sale starts

• B) Allows free mints after ~13337 blocks from when startSale() is called

• C) Visibility should be changed to external

• D) None of the above

Click the reveal the answer

A,B

---

Q4 InSecureumNFT contract is

• A) Not susceptible to reentrancy given the absence of external contract calls

• B) Not susceptible to integer overflow/wrapping given the compiler version used and the absence of unchecked blocks

• C) Susceptible to reentrancy during minting

• D) Perfectly safe for production

Click the reveal the answer

B,C

---

Q5 Assuming InSecureumNFT contract is deployed in production (i.e. live for users) on mainnet without any changes to shown code

• A) Use of evident test configuration will cause fewer NFTs to be minted than expected in production

• B) Illustrates the lack of best-practice for test parameterization to be removed or kept separate from production code

• C) It will behave as documented in code to mint the expected number of NFTs in production

• D) None of the above

Click the reveal the answer

A,B

---

Q6 The function startSale()

• A) May be successfully called/executed by anyone

• B) May be successfully called/executed with _price of 0

• C) Must be called for minting to happen successfully

• D) None of the above

Click the reveal the answer

A,B,C

---

Q7 The minting of NFTs

• A) Requires an exact amount of ETH to be paid by the buyer

• B) Refunds excess ETH paid by buyer back to the buyer

• C) Transfers the NFT salePrice to the beneficiary address

• D) May be optimized to prevent any zero ETH transfers in its refund mechanism

Click the reveal the answer

B,C,D

---

Q8 The NFT sale

• A) May be restarted by anyone any number of times

• B) Can be started exactly once by deployer

• C) Is missing an additional check on publicSale

• D) Is missing an event emit in startSale

Click the reveal the answer

s

A,C,D