Description

In the receiveAssetsFromCegaState() function, the following code is used to determine whether the vaultAddress passed to it corresponds to a valid vault:

function receiveAssetsFromCegaState(address vaultAddress, uint256 amount) public {
    require(msg.sender == address(cegaState), "403:CS");
    FCNVaultMetadata storage vaultMetadata = vaults[vaultAddress];
    // a valid vaultAddress will never have vaultStart = 0
    require(vaultMetadata.vaultStart != 0, "400:VA");

    IERC20(asset).safeTransferFrom(msg.sender, address(this), amount);
    vaultMetadata.currentAssetAmount += amount;
}

This check looks correct at first glance because the only way to get a vault metadata's vaultStart property set is through the createVault() function, which always creates an instance of an FCNVault contract:

function createVault(
    string memory _tokenName,
    string memory _tokenSymbol,
    uint256 _vaultStart
) public onlyTraderAdmin returns (address vaultAddress) {
    require(_vaultStart != 0, "400:VS");
    FCNVault vault = new FCNVault(asset, _tokenName, _tokenSymbol);
    address newVaultAddress = address(vault);
    vaultAddresses.push(newVaultAddress);

    // vaultMetadata & all of its fields are automatically initialized if it doesn't already exist in the mapping
    FCNVaultMetadata storage vaultMetadata = vaults[newVaultAddress];
    vaultMetadata.vaultStart = _vaultStart;
    vaultMetadata.vaultAddress = newVaultAddress;

    emit VaultCreated(newVaultAddress, vaultAddresses.length - 1);
    return newVaultAddress;
}

However, the rolloverVault() function can allow a malicious or compromised trader admin to bypass this check. The rolloverVault() function is missing a check to ensure that the vaultAddress passed to it is valid:

function rolloverVault(address vaultAddress) public onlyTraderAdmin {
    FCNVaultMetadata storage vaultMetadata = vaults[vaultAddress];
    require(vaultMetadata.vaultStatus == VaultStatus.WithdrawalQueueProcessed, "500:WS");
    require(vaultMetadata.tradeExpiry != 0, "400:TE");
    vaultMetadata.vaultStart = vaultMetadata.tradeExpiry;
    // [ ... ]
}

This can be used to set an arbitrary address's vaultStart metadata property to a non-zero value, which would bypass the vaultAddress validity check.

Impact

A malicious or compromised trader admin can cause the vault metadata of an arbitrary address to look like a valid FCNVault.

First, a malicious vault contract must be created. It must contain the following functions:

1. A totalSupply() function that returns a value greater than zero.

2. An empty deposit() function.

3. An empty redeem() function.

Then, the malicious or compromised trader admin can take the following steps to set the malicious vault contract's vaultStart metadata property to a non-zero value.

1. Call openVaultDeposits() with the malicious vault address. This will set the vault's status to DepositsOpen.

2. Call processDepositQueue() with the malicious vault address. This will set the vault's status to NotTraded.

3. Call setTradeData() with the _tradeExpiry set to a non-zero value, such that it is set to a time in the past.

4. Call sendAssetsToTrade() with the amount set to 0. This sets the vault's status to Traded.

5. Call calculateCurrentYield() with the malicious vault address. This will set the vault's status to TradeExpired.

6. Call calculateVaultFinalPayoff() with the malicious vault address. This will set the vault's status to PayoffCalculated.

7. Call collectFees() with the malicious vault address. This will set the vault's status to FeesCollected.

8. Call processWithdrawalQueue() with the malicious vault address. The withdrawal queue is empty, so this will just set the vault's status to WithdrawalQueueProcessed.

9. Call rolloverVault() with the malicious vault address. Both the require statements in the function will pass, and the vaultStart metadata property will be set to the _tradeExpiry value from step 3.

Recommendations

Add the following check to rolloverVault():

function rolloverVault(address vaultAddress) public onlyTraderAdmin {
    FCNVaultMetadata storage vaultMetadata = vaults[vaultAddress];
    require(vaultMetadata.vaultStart != 0); // Add this check

    // [ ... ]
}

Remediation

The client has acknowledged and fixed this issue by adding a vault address validity check to rolloverVault(). This was fixed in commit f64513a9↗.