Sandbox L2

Both functions `batchTransferQuad()` and `transferQuad()` call the internal function `_transferQuad()` to transfer quads. If the quad being transferred is 1x1, then the token is checked to ensure that it is not burned. However, when the quad is of a larger size, no such check is made, meaning that tokens with the burnt flag can have the flag reset. Each token is associated to a 256 bit `uint` in the mapping `_owners`. The least significant 160 bits are used for the token owner's address while the least significant 161st bit is used to decide if the token is burned or not. When using `batchTransferQuad()` or `transferQuad()` to transfer a quad of size larger than 1x1, the internal function `_regroup()` is called to perform checks and execute the transfer. The function `_regroup()` first checks if each 1x1 in the quad is owned by the correct owner for the transfer via `_checkAndClear()`, but this check only considers if the owner is non-zero and if so, the least significant 160 bits. ```solidity=534 function _checkAndClear(address from, uint256 id) internal returns (bool) { uint256 owner = _owners[id]; if (owner != 0) { require(address(uint160(owner)) == from, "not owner"); _owners[id] = 0; return true; } return false; } ``` Note that `_checkAndClear()` sets the owner of the 1x1 to 0 if the checks are passed. As burned tokens may retain information about the previous owner, such as if a user transferred a 1x1 LAND to themselves and then burned the 1x1, they would be able to clear the burn flag by calling `transferQuad()` to transfer the 1x1 to themselves. However, this process decreases the user's balance due to the burn, so resetting the burn flag can only be done if the user has an excess balance amount. Another issue of this is that it is possible for users to be unable to transfer quads. The reason is that the burning process decreases their balance. For example, if a user minted themselves a 6x6 quad, their current balance is 36. Suppose the user transfers a 1x1 LAND to themselves and then burns it, bringing their balance to 35. The user may forget this occurred and if they sell a 3x3 quad containing the burnt land, their new balance would be 26, even though they are the owner of 27 pieces of LAND. Hence they will be unable to sell all of their tokens due to an underflow revert. **Note: the detailed proof of concept can be found in *Appendix - Supplementary Tests: Test 1 & Test 2*.**

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.