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#L289

Vulnerability details

Note: This submission contains links to a private fork of the contest repo. User code423n4 has been added as a collaborator in order to view.

Impact

In a fully decentralised setting users must also be wary of the smart contract owner and the possibility of "rug pulls". Their trust can be increased by limiting the contract owner's power. In the Cally contract the contract owner is able to set the fee rate arbitrarily high. For values strictly greater than 100% the contract will revert on all calls to exercise leading to a denial of service. However, entering such a value is unlikely and is easily fixed.

However, the fee rate can be set as high at 100% (i.e. 1e18) while still retaining contract functionality. On a call to exercise by an Option holder this means that the vault beneficiary receives no ether! See Cally.sol:289.

Although the feeRate is public, setFee can be called at any time, including after the option has been bought with buyOption.

The impact is that Option sellers only receive the premium but not the strike price. Considering that the strike price is many times larger than the premium this is a substantial loss, and this loss is the contract owner's gain.

Proof of Concept

A Foundry test has been written that shows how the contract owner can rug pull by setting the fee rate to 100%.

The steps are as follows:

Fee is initially zero
User babe creates a vault
User alice buys the option
User babe sees she got the premium
Contract owner calls setFee with 1e18 as argument (i.e. 100%)
User alice exercises the option
User babe checks her balance and sees that her ethBalance has not increased!
Contract owner checks protocolUnclaimedFees and sees it has increased by strike price!

Tools Used

Manual inspection

Recommended Mitigation Steps

Add a public constant to the Cally contract that sets a maximum fee rate. For example:

uint256 public constant MAX_FEE_RATE = 5e16; // 0.05%

Then re-implement setFee as follows:

function setFee(uint256 feeRate_) external onlyOwner {
    require(feeRate_ <= MAX_FEE_RATE, "Fee rate is too high!");
    feeRate = feeRate_;
}

An implementation in a private fork appears here and here.

A test confirming that the contract owner can't set a fee rate that is too high appears here.

Also, the function setFee should be called setFeeRate as it is a more accurate name.

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