Cally contest - Cityscape's results

Judge has assessed an item in Issue #98 as Medium risk. The relevant finding follows:

Set Limits on setFee() A Malicious owner could set feeRate to = (100 * 1e18) / 100; which would give the entire value of an exercise() transaction to the protocol, create a limit on the fees the owner can set.

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L119-L121

Non-Critical

Missing returns

return natspec is missing for the following functions

• withdrawProtocolFees
• createVault
• buyOption
• harvest
• vaults

Typo in Comment

// burns a token without checking owner address is not 0

proposed change:

// burns a token without checking if owner address is not 0

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/CallyNft.sol#L23

Low-Risk

Use _safeMint() instead of _mint()

_safeMint() is preferred to _mint() as stated here openzeppelin-contracts/ERC721.sol at d4d8d2ed9798cc3383912a23b5e8d5cb602f7d4b · OpenZeppelin/openzeppelin-contracts · GitHub, _safeMint ensures the recipient is either an EOA or implements IERC721Receiver.

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L193

Set Limits on setFee()

A Malicious owner could set feeRate to = (100 * 1e18) / 100; which would give the entire value of an exercise() transaction to the protocol, create a limit on the fees the owner can set.

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L119-L121

Possible to submit ERC-777 as token param in createVault leading to possible re-entrancy attack

If an erc-777 token address is put in token parameter and tokenType is set to either ERC721 or ERC20 in createVault() then it is possible to submit this token to the protocol leading to a potential vector for re-entrancy attack.

Gas Optimizations

Unnecessarily initialized Variable

uint256 fee = 0;

proposed change:

uint256 fee;

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L90

uint256 public protocolUnclaimedFees = 0;

proposed change:

uint256 public protocolUnclaimedFees;

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L92

uint256 public feeRate = 0;

proposed change:

uint256 public feeRate;

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L282

---

Set Array Sizes

uint256[] public premiumOptions = [0.01 ether, 0.025 ether, 0.05 ether, 0.075 ether, 0.1 ether, 0.25 ether, 0.5 ether, 0.75 ether, 1.0 ether, 2.5 ether, 5.0 ether, 7.5 ether, 10 ether, 25 ether, 50 ether, 75 ether, 100 ether];
uint256[] public strikeOptions = [1 ether, 2 ether, 3 ether, 5 ether, 8 ether, 13 ether, 21 ether, 34 ether, 55 ether, 89 ether, 144 ether, 233 ether, 377 ether, 610 ether, 987 ether, 1597 ether, 2584 ether, 4181 ether, 6765 ether];

proposed change:

uint256[17] public premiumOptions = [0.01 ether, 0.025 ether, 0.05 ether, 0.075 ether, 0.1 ether, 0.25 ether, 0.5 ether, 0.75 ether, 1.0 ether, 2.5 ether, 5.0 ether, 7.5 ether, 10 ether, 25 ether, 50 ether, 75 ether, 100 ether];
uint256[19] public strikeOptions = [1 ether, 2 ether, 3 ether, 5 ether, 8 ether, 13 ether, 21 ether, 34 ether, 55 ether, 89 ether, 144 ether, 233 ether, 377 ether, 610 ether, 987 ether, 1597 ether, 2584 ether, 4181 ether, 6765 ether];

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L90-L92

---

Use & 1 != 0 instead of % 2 != 0

require(vaultId % 2 != 0, "Not vault type");

proposed change:

require(vaultId & 1 != 0, "Not vault type");

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L211

require(vaultId % 2 != 0, "Not vault type");

proposed change:

require(vaultId & 1 != 0, "Not vault type");

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L304

Read direct from storage

function getPremium(uint256 vaultId) public view returns (uint256 premium) {
        Vault memory vault = _vaults[vaultId]; 
        return premiumOptions[vault.premiumIndex];    
}

proposed change:

function getPremium(uint256 vaultId) public view returns (uint256 premium) {
        return premiumOptions[_vaults[vaultId].premiumIndex];
    }

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L394-L397

Using both named returns and return is not necessary

function getPremium(uint256 vaultId) public view returns (uint256 premium)
function getVaultBeneficiary(uint256 vaultId) public view returns (address beneficiary)

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L394

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L378