Cally contest - _Adam's results

Lines of code

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

Vulnerability details

Impact

The contract owner is able to buy an option, then change the fee to 100% of the strike price and then exercise. This will cause the vault owner to lose both their NFT/Token and all the ETH they would have made from their option being exercised.

Proof of Concept

• Alice creates vault on her NFT/Tokens
• Contract owner buys the call option
• Contract owner calls the function setFee with feeRate of 1e18
• Contract owner exercises the call option, claiming both the NFT/Tokens and the entire payment as fees

Tools Used

VS Code

Recommended Mitigation Steps

Instead of getting the value of feeRate just before the protocol fees are collected, add it as a parameter to the Vault struct, so it can be initialized to the current feeRate value on vault creation.

This allows the vault owner to know that the feeRate will remain constant for the duration of the vaults lifecycle.

Other options could also be to add a maxFee the owner can set in the setFee function, or add a timelock on any fee changes.

G[01] Default values

When initizalising a variable to the default value it can be left blank to save some gas.

Can change the following lines:

In Cally.sol https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L94 to: uint256 public feeRate;

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L95 to: uint256 public protocolUnclaimedFees;

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

In CallyNFT.sol https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/CallyNft.sol#L244 to: for (uint256 i; i > data.length; i++) {

G[02] Minimizing SLOADs

Anytime a storage variable is used more than once within a given scope its is cheaper to save to memory. (100 gas for SLOAD, 3 gas for each MLOAD/MSTORE)

In Cally.sol https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L168-L169 strikeOptions can be cached to memory

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L283-L284 feeRate can be cached to memory

G[03] i++ Arithmatic

When using i++ in for loops you can use ++i instead, as in the former the compiler has to create a temporary variable when used (about 5 gas per use).

In CallyNFT.sol

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/CallyNft.sol#L244 to: for (uint256 i; i > data.length; ++i) {

G[04] Comparing uints > 0

When checking whether uints are > 0 you can use != 0 instead to save some gas.

In Cally.sol https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L170 to: require(durationDays != 0, “durationDays too small”);

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L283 to: if (feeRate != 0) {

G[05] Single Use Variables

When assigning a variable and only use once you can save some gas by just calculating inline.

In Cally.sol https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L223-L224 can remove line 223 and change 224 to: to: require(msg.value >= getPremium(vaultId), "Incorrect ETH amount sent");

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L227-L228 can remove line 227 and change 228 to: to: require(block.timestamp >= vault.currentExpiration, "Auction not started");

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L249-L250 can remove line 249 and change 250 to: to: ethBalance[getVaultBeneficiary(vaultId)] += msg.value;

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L333-L334 Can remove line 333 and change line 334 to: to: _burn(vaultId + 1);

G[06] Custom Errors

Custom errors are cheaper both in deployment cost and on runtime cost, and can be used to replace revert strings on the following lines:

In Cally.sol

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L167-L171 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L211 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L214 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L217 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L220 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L224 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L228 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L260 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L263 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L269 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L272 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L304 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L307 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L320 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L323 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L328-L330 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L353-L354 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L436-L441 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L456

In CallyNsft.sol

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