code-423n4 / 2022-05-cally-findings

2 stars 0 forks source link

Admin can change fee parameter at any time. Fee parameter is ubounded #211

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

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.
outdoteth commented 2 years ago

owner can change fee at any time; https://github.com/code-423n4/2022-05-cally-findings/issues/47 owner can set fee greater than 100%: https://github.com/code-423n4/2022-05-cally-findings/issues/48