Description

Investors in the Cega protocol use the FCNProduct contract's addToDepositQueue() function to deposit their funds. This function uses safeTransferFrom() to transfer asset tokens from the investor's address to the FCNProduct contract.

function addToDepositQueue(uint256 amount, address receiver) public {
    require(isDepositQueueOpen, "500:NotOpen");
    queuedDepositsCount += 1;
    queuedDepositsTotalAmount += amount;
    require(queuedDepositsTotalAmount + sumVaultUnderlyingAmounts <= maxDepositAmountLimit, "500:TooBig");

    IERC20(asset).safeTransferFrom(receiver, address(this), amount);
    depositQueue.push(Deposit({ amount: amount, receiver: receiver }));
    emit DepositQueued(receiver, amount);
}

Once these funds are deposited, the only way for the funds to leave the contract are through the following functions:

1. collectFees() - Only callable by the trader admin. Used to collect fees for the Cega protocol.

2. processWithdrawalQueue() - Only callable by the trader admin. Used to process investor withdrawals.

3. sendAssetsToTrade() - Only callable by the trader admin. Used to send deposited assets to a market maker.

As there are no other ways to take deposited funds out of the contract, a malicious or compromised trader admin may choose simply to not call any of these functions. If this were to happen, any deposited investor funds (and any other funds in the contract) will become locked in the contract forever.

Impact

A compromised or malicious trader admin can lead to funds being locked in the FCNProduct contract forever.

Recommendations

Consider adding a sweep-style function that allows the protocol to transfer out any tokens in the contract to a chosen address. Ideally, this function should only be accessible by the default admin multi-sig role.

function sweepTokens(address receiver) external onlyDefaultAdmin {
    IERC20(asset).safeTransfer(receiver, IERC20(asset).balanceOf(address(this)));
}

Remediation

The client has acknowledged this issue, and has stated that it is mitigated due to the fact that the DefaultAdmin role can assign a new TraderAdmin through the CegaState contract.