Description

The Cega smart contracts rely on a number of protocol parameters to function correctly. There are functions that allow the admins of the protocol to alter most of these parameters.

We found that the majority of these parameters are not checked to be within certain limits before they are set. The majority of these setter functions are only accessible by either the operator admin or the trader admin, both of which are non--multi-sig wallets.

In particular, the following functions are only accessible by either the operator admin or the trader admin. They are missing sanity checks on crucial protocol parameters before they are set:

1. setManagementFeeBps() - If set to 100%, it could lead to all investor funds being sent to the Cega fee recipient. Only accessible by the operator admin.

2. setYieldFeeBps() - Similar to setManagementFeeBps(). Only accessible by the operator admin.

3. setMaxDepositAmountLimit() - If set to 0, it will prevent the FCNProduct contract from accepting deposits, leading to denial of service. Only accessible by the trader admin.

4. setTradeData() - If the tradeExpiry parameter is set to hundreds of years in the future, funds would effectively be locked forever in the vault. Furthermore, the aprBps parameter can be set to a very high number. The trader admin can become an investor themselves and profit off of the high APR. Only accessible by the trader admin.

5. updateOptionBarrierOracle() and addOracle() - Allows full control over which oracle is used for a specific option barrier. Only accessible by the operator admin.

6. addOptionBarrier() and updateOptionBarrier() - If the strikeAbsoluteValue is set to 0, then a revert will occur when calculateVaultFinalPayoff() is called, as it will result in a division by 0 in calculateKnockInRatio(). Only accessible by the trader admin.

Impact

If the trader admin or operator admin roles are ever compromised or turn malicious, they would be able to set crucial protocol parameters to arbitrary values. This would cause the smart contracts to function incorrectly, and it may lead to loss of protocol or investors' funds in the worst case.

Specifically, consider the setTradeData() function, which allows trader admins to modify vault-specific metadata parameters. This function does not check the validity of the parameters. So, if a trader admin were to set the tradeExpiry parameter to a non-zero value that is less than the vaultStart configured in the createVault function, the collectFees() function would not be callable (i.e., the trader admin would be locked out of collecting fees).

The collectFees() function internally calls the calculateFees() function, which has the following subtraction that would underflow:

function calculateFees(
    FCNVaultMetadata storage self,
    uint256 managementFeeBps,
    uint256 yieldFeeBps
) public view returns (uint256, uint256, uint256) {
    // [...]
    uint256 numberOfDaysPassed = (self.tradeExpiry - self.vaultStart) / SECONDS_TO_DAYS;
    // [...]
}

Note that these parameters can be overridden by the default admin role (which is intended to be controlled by a multi-sig) using the setVaultMetadata() function, and therefore the impact is partially mitigated.

Recommendations

Add sanity checks to these functions to ensure the parameters are within sane limits.

Remediation

The client has acknowledged and fixed this issue by adding the necessary sanity checks. This was fixed in commit a638c874↗.