# Jake's Audit Notes: Zora

Client: Zora https://zora.co/

https://github.com/ourzora/core/releases/tag/v1.0.0-rc8 `7964c8c`

## Whitepaper & specification about the project

Whitepaper: https://docs.zora.co/zoraos/dev/smart-contracts/whitepaper

Project is an NFT market place.

## Review of the protocol/implementation

**[1] Missing Media Contract Event**

**Files Affected:** [`contracts/Market.sol`](https://github.com/ourzora/core/blob/7964c8c6b8a7d387db93cdcea0641cbae772e303/contracts/Market.sol)

**Severity: Low**

[`configure()`](https://github.com/ourzora/core/blob/7964c8c6b8a7d387db93cdcea0641cbae772e303/contracts/Market.sol#L203) does not emit an event when the contract address is set. As this address is the only one that may call mutable functions, it would be useful to know if it has been set correctly.

**Recommendations:**

Emit an event when the `mediaContractAddress` has been set.

**[2] Mapping Value Not Checked For Existence**

**Severity: Low**

**Files affected:** [`contracts/Market.sol`](https://github.com/ourzora/core/blob/7964c8c6b8a7d387db93cdcea0641cbae772e303/contracts/Market.sol)

[`setBid()`](https://github.com/ourzora/core/blob/7964c8c6b8a7d387db93cdcea0641cbae772e303/contracts/Market.sol#L274) does not check if the mapping element exists.

**Recommendations:**

Add a require statement that verifies the Token ID exists within the mapping:

`require (_bidShares[tokenId].active);`

**[3] Currency Not Checked for Validity**

**Severity: Low**

**Files affected:** [`contracts/Market.sol`](https://github.com/ourzora/core/blob/7964c8c6b8a7d387db93cdcea0641cbae772e303/contracts/Market.sol#L265)

[`setBid()`](https://github.com/ourzora/core/blob/7964c8c6b8a7d387db93cdcea0641cbae772e303/contracts/Market.sol#L274) does not check if the function argument of the currency passed.

**Recommendations:**

Add a require statement that verifies currency passed is not the zero address.

`require(bid.currency!= address(0), "Market: Currency cannot be 0 address");`

**[4] Mint function susceptible to reentrancy**

**Severity: Medium**

**Files affected:** [`/contracts/Media.sol`](https://github.com/ourzora/core/blob/7964c8c6b8a7d387db93cdcea0641cbae772e303/contracts/Media.sol#L236)

[`mint()`](https://github.com/ourzora/core/blob/7964c8c6b8a7d387db93cdcea0641cbae772e303/contracts/Media.sol#L236) is able to be called to mint new media from a bid. No protection is in place to prevent multiple instances of this function being called.

**Recommendations:**

Add `nonReentrant` to the function to prevent multiple minting of media from the same bid.

**[5] Broken exchange logic**

**Severity: Medium**

**Files affected:** `/contracts/Market.sol`

From the documentation: *whenever a bid is accepted, or an ask fulfilled, the piece of media is transferred to the buyer.*

`setAsk()` does not check if there’s a counterpart bid matching the ask amount. This means bids will not be matched in this scenario:
1) A seller lists media for the token.

2) A bidder submits a bid for the token at a lower price than the ask.

3) It is not possible to change existing asks or bids. They must be removed and re-added. A seller removes the ask.

4) The seller re-adds the ask with a price for which there is a bid from the previous listing. It is not matched because [`setAsk()`](https://github.com/ourzora/core/blob/7964c8c6b8a7d387db93cdcea0641cbae772e303/contracts/Market.sol#L237) does not have the same logic as `setBid()` to check for the counterparty side of the trade, so the trade does not execute.

**[6] Bid currency does not take dynamic supply currencies into account**

**Severity: Low**

**Files affected:** [`contracts/Market.sol`](https://github.com/ourzora/core/blob/7964c8c6b8a7d387db93cdcea0641cbae772e303/contracts/Market.sol#L265)

Does not take into account for rebase/dynamic supply tokens such as Ampleforth. This could affect accounting where the tokens supplied do not exist because of a shrunken market cap.

**Recommendations:**

Add support for dynamic supply currencies, or prevent them from being used as a bid share.

**[7] Unimplemented split of the bid**

**Severity: Medium**

**Files affected:** [`contracts/Media.sol`](https://github.com/ourzora/core/blob/7964c8c6b8a7d387db93cdcea0641cbae772e303/contracts/Media.sol)

The documentation states: *The split of the bid can be unique to each piece of media. For example, assume a creator mints a piece of media specifying the creator share to be 5%. Upon each sale, 5% of the bid’s value is transferred to the owner, and 95% of the bid’s value is transferred between the owner and previous owner, depending on any sell-on fee defined by the previous sale of the piece.*

[`mint()`](https://github.com/ourzora/core/blob/7964c8c6b8a7d387db93cdcea0641cbae772e303/contracts/Media.sol#L236) does not allow the creator to specify the share so it is not possible to split the bid with a percentage.

**Recommendations:**

Implement this functionality from the documentation.

**[8] Incorrect implementation of the media URI generation**

**Severity: Medium**

**Files affected:** [`contracts/Media.sol`](https://github.com/ourzora/core/blob/7964c8c6b8a7d387db93cdcea0641cbae772e303/contracts/Media.sol)

[`TokenURI()`](https://github.com/ourzora/core/blob/7964c8c6b8a7d387db93cdcea0641cbae772e303/contracts/Media.sol#L195) attempts to access a mapping `_tokenURIs` that does not exist. This means that the URI for the media will not be returned.

**Recommendations:**

Implement the functionality to return the correct URI for the media from the token ID.

## Best Practices

- Tests don't run with documentation in the README.md
- Fix linter issues:

BaseErc20.sol
  4:2  error  Line length must be no more than 120 but current length is 131  max-line-length

Decimal.sol
  18:2  error  Line length must be no more than 120 but current length is 130  max-line-length
  21:2  error  Line length must be no more than 120 but current length is 132  max-line-length

ERC721.sol
    4:2  error  Line length must be no more than 120 but current length is 144  max-line-length
   10:2  error  Line length must be no more than 120 but current length is 127  max-line-length
   11:2  error  Line length must be no more than 120 but current length is 171  max-line-length
   12:2  error  Line length must be no more than 120 but current length is 148  max-line-length
  335:2  error  Line length must be no more than 120 but current length is 136  max-line-length
  392:2  error  Line length must be no more than 120 but current length is 136  max-line-length

ERC721Burnable.sol
  4:2  error  Line length must be no more than 120 but current length is 152  max-line-length

Market.sol
  86:2  error  Line length must be no more than 120 but current length is 126  max-line-length
  87:2  error  Line length must be no more than 120 but current length is 123  max-line-length
  88:2  error  Line length must be no more than 120 but current length is 123  max-line-length

Media.sol
  61:2  error  Line length must be no more than 120 but current length is 125  max-line-length

✖ 14 problems (14 errors, 0 warnings)