Cally contest - throttle's results

Lines of code

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

Vulnerability details

Impact

Admin can change fee parameter at any time.

Proof of Concept

First of all, the fee parameter is unbounded. It can be as high as 100%. https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L117-L121

Additionally, an admin can change fee parameter for any sale at any time. For example, an admin can change the fee after the vault is created or call option is bought. And before call option is exercised.

POC:

1. Alice sees that the fee is 0%. She creates a vault with very popular and valuable NFT (price 100 ETH)
2. Bob, the trader buys the call option.
3. Admin changes fee to 30% (or 100% if admin being malicious).
4. NFT market booming. NFTs are even more valuable. Bob decided to exercise his option.
5. Alice gets only 70 ETH (0 ETH if admin being malicious).

Tools Used

Manual review

Recommended Mitigation Steps

1. Bound fee change. For example < 20%
2. Store fee parameter in a vault struct during vault creation and use that fee for accounting.

[L-01] Payment excess is not given back

Description

Trader can overpay for the call option premium. https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L224

Recommended Mitigation Steps

Return excess to the rightful user.

-----------------------------------------------------------------