NonUSTStrategy: Ensure correct UST index

[code423n4]

Handle

hickuphh3

Vulnerability details

Impact

It is crucial to verify the correctness of the UST index passed in _ustI. Otherwise, a malicious party can cause funds to be exchanged to a stablecoin that is neither UST nor the underlying token. Since there is no way to exchange it back to the underlying nor rescue the funds, all converted funds would be permanently trapped in the strategy contract.

For now, the proposed attack vector is not feasible because it requires a pool of minimally 3 indexes, while the UST curve pools with good liquidity are the UST-3CRV and UST-MIM pools which have only 2 indexes each.

Proof of Concept

1. Assume the following:
   • existing curve pool of sUSD, UST and MIM is seeded with sufficient liquidity
   • Vault underlying token is MIM
   • index of sUSD, UST and MIM are 0, 1 and 2 respectively
2. Strategy incorrectly uses 0 as the _ustI value and goes unnoticed. A malicious party notices the error and would like to cause griefing. The malicious party sends 1 wei of UST to the strategy. This is to ensure that the non-zero UST balance check in _initDepositStable() passes.

function _initDepositStable() internal {
  uint256 ustBalance = _getUstBalance();
  require(ustBalance > 0, "balance 0");
  ...
}

1. updateInvested() is called, sending the MIM funds to the strategy contract and converted presumably to UST. However, they are converted to sUSD instead. There is no way to convert sUSD back to UST because no allowance is given to the curve pool. There is also no function that allows for funds to be rescued.

Recommended Mitigation Steps

It will probably be good to verify the correctness of the _ustI index. This has an additional benefit of verifying the correctness of the curve pool used.

require(ICurve(_curvePool).coins(_ustI) == ustToken, "incorrect _ustI or pool");

[naps62]

Fixed in https://github.com/lindy-labs/sc_solidity-contracts/pull/69