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

2 stars 0 forks source link

Owner of Cally.sol can set arbitrarily big fees, over 100% and steal the funds. #249

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

The fee rate is set by the owner and doesn't have any restrictions. The owner can set fees over 100% and steal the funds.

Proof of Concept

The fee rate setted by the owner without restrictions. https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L119

When a user wants to exercise the right of the call, fees are charged over the amount sended to buy the option underlying asset.

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

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

If the rate is 1e18 the owner steals all the funds from the caller of exercise(), if it's bigger he can steal all the other funds of the contract. Even if is set to very big values, protocolUnclaimedFees could be bigger than the balance of the contract and it would be impossible to withdraw the funds. https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L124

Recommended Mitigation Steps

Set a max for fee rates.

outdoteth commented 2 years ago

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