The Sandbox security record

The report gives no description here.

The report gives no description here.

The report gives no description here.

The report gives no description here.

The report gives no description here.

The report gives no description here.

The NatSpec of the `_mintQuad()` function states that mints a quad of size 3, 6, 12, or 24 only. ```solidity=111 * @notice Mint a new quad (aligned to a quad tree with size 3, 6, 12 or 24 only) ``` However, the function implementation allows mints of size 1. ```solidity=117 function _mintQuad( address to, uint256 size, uint256 x, uint256 y, bytes memory data ) internal { require(to != address(0), "to is zero address"); require(!exists(size, x, y), "Already minted"); uint256 quadId; uint256 id = x + y * GRID_SIZE; if (size == 1) { quadId = id; ``` **Proof of Concept** ```typescript describe('Can mint 1x1 Land with mintQuad function', function () { it('Invoke mintQuad() to mint 1x1 land', async function() { const {landOwners} = await setupTest(); const bytes = '0x3333'; await waitFor( landOwners[0].MockLandWithMint.mintQuad( landOwners[1].address, 1, 0, 0, bytes ) ); const owner = await landOwners[0].MockLandWithMint.ownerOf(0) await expect(owner).to.equal(landOwners[1].address); console.log('Successfully mint a 1x1 land at (0,0)') }); }); ``` result: ```typescript Successfully mint a 1x1 land at (0,0) ✓ Invoke mintQuad() to mint 1x1 land (6578ms) ```

The `approveFor()` and `approve()` functions are used to approve an operator to spend tokens on a user's behalf, but they are inconsistent with each other as it is possible to approve a burnt token using `approveFor()` but not `approve()`. When `approve()` is used, the owner of the token is acquired by calling `_ownerOf()`, which will return the zero address for a burnt token, and a check is done to ensure that the owner is not the zero address. ```solidity=35 function approve(address operator, uint256 id) external override { uint256 ownerData = _owners[_storageId(id)]; address owner = _ownerOf(id); address msgSender = _msgSender(); require(owner != address(0), "NONEXISTENT_TOKEN"); ``` Hence it is not possible to approve a burnt token. However, for the function `approveFor()`, the owner is acquired from the `_owners` mapping and the check on this owner is that the least significant 20 bytes is the same as the `sender`. ```solidity=51 function approveFor( address sender, address operator, uint256 id ) external { uint256 ownerData = _owners[_storageId(id)]; address msgSender = _msgSender(); require(sender != address(0), "ZERO_ADDRESS_SENDER"); require( msgSender == sender || _superOperators[msgSender] || _operatorsForAll[sender][msgSender], "UNAUTHORIZED_APPROVAL" ); require(address(uint160(ownerData)) == sender, "OWNER_NOT_SENDER"); ``` As a burnt token retains information about the previous owner, it is possible to pass all of these checks. A consequence of this is that after approving the burnt token, its associated value in the `_owners` mapping will be `BURNED_FLAG = 2**160`, losing the information of the previous owner. **Note: the detailed proof of concept can be found in *Appendix - Supplementary Tests: Test 3*.**

The `exists()` function is used to see if an input quad of size `size` starting at coordinates `(x,y)` is contained in or contains an already minted quad. It returns a bool value: - `true`: the input quad contains an already minted quad (meaning you cannot mint the quad) - `false`: the input quad does not contain an already minted quad (meaning you may mint the quad) However, this function does not ensure that `size` is a parameter supported by the project. Although this function is only called in `_mintQuad()` where `size` is ensured to be an input supported by the project, `exists()` is a `public` function, meaning a user may use this on unsupported parameters for `size`, which may lead to incorrect return values. For example, suppose a user wants enough LAND to cover a 13x13 square starting at (13,0). Assume (12,0) is owned as a 12x12 quad and no one owns a 24x24 quad at (0,0). Now suppose the user calls `exists(13,13,0)` to see if all LANDs and quads are free to mint. The 24x24 check passes since no one owns the 24x24. As `size` is larger than 12, the function checks if anyone owns a 12x12 starting at (13,0), which is not possible. Similarly, the 6x6, 3x3, and 1x1 checks can be passed so it returns `false` even though pieces of the 13x13 grid are owned. **Proof of Concept**\ The following test is derivated from the example above: ```typescript it('Exists function can be misleading', async function () { const {landOwners} = await setupTest(); const bytes = '0x3333'; await waitFor( landOwners[0].MockLandWithMint.mintQuad( landOwners[0].address, 12, 12, 0, bytes )); const owner = await landOwners[0].MockLandWithMint.ownerOf(12); await expect(owner).to.be.equal(landOwners[0].address); const existResult = await landOwners[0].MockLandWithMint.exists(13,13,0); await expect(existResult).to.be.equal(false); }); }); ``` The result shows it returns `false`, which could be misleading to front end/users. ```typescript Possibly Incorrect Return for Function exists() Nothing to compile ✓ Exists function can be misleading (6426ms) ```

The internal function `_ownerOfQuad()` is used to find the owner or parent owner of a quad and can only be called by itself or one of the regroup functions. As none of these will call `_ownerOfQuad()` with a value of 1 for the input variable `size`, the `if` code branch `size == 1` will never be reached.