Description

The lending storage manager includes a function to migrate the multiple liquidity pool to a new address; this function can only be called by the multiple liquidity pool. The migration function does not migrate critical accounting information such as the total number of synthetic tokens or the collateral assets of the liquidity providers.

function migratePool(address oldPool, address newPool)
  external
  override
  nonReentrant
  onlyPoolFactory
{
  ...
  // copy storage to new pool
  newPoolData.lendingModuleId = oldLendingId;
  newPoolData.collateral = oldPoolData.collateral;
  newPoolData.interestBearingToken = oldPoolData.interestBearingToken;
  newPoolData.jrtBuybackShare = oldPoolData.jrtBuybackShare;
  newPoolData.daoInterestShare = oldPoolData.daoInterestShare;
  newPoolData.collateralDeposited = oldPoolData.collateralDeposited;
  newPoolData.unclaimedDaoJRT = oldPoolData.unclaimedDaoJRT;
  newPoolData.unclaimedDaoCommission = oldPoolData.unclaimedDaoCommission;
  ...
}

The following critical accounting information in the pool is not migrated:

contract SynthereumMultiLpLiquidityPool...
  uint256 internal totalSyntheticAsset;
  ...
  mapping(address => LPPosition) internal lpPositions;
  ...

Impact

The multiple liquidity pool currently does not implement a function calling the pool migration function; however, implementing a function calling the migration function in its current state would result in lost funds.

Recommendations

We recommend removing the function until the implementation is corrected. We further note that fixing these issues will require more than just changing the migratePool(...) function in the lending storage manager; it will also require changes to be made in the multiple liquidity pool to update the fields totalSyntheticAsset and read and update the lpPositions mapping.

Remediation

Jarvis has made considerable efforts to address the concerns conveyed in this finding. They have created a library for managing the pool migration, which appears to address the main concerns of (1) migrating LP-level collateral and token assets and (2) migrating total pool synthetic tokens. It is important to note, however, that this migration contract lies outside of the core scope of this audit and has hence not received the same level of scrutiny as the rest of the contracts. Furthermore, we have not been presented with an updated multiple liquidity pool contract that utilizes this library for pool migrations. Jarvis appears to be on the right track here, and we look forward to seeing a completed and safely implemented pool migration function in the future.