search

code-423n4 / 2023-06-lybra-findings

8 stars 7 forks source link

`LybraConfigurator::getBadCollateralRatio` will underflow if `vaultSafeCollateralRatio[pool]` is not set #513

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/configuration/LybraConfigurator.sol#L338-L341 https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/configuration/LybraConfigurator.sol#L331-L336 https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L125-L129 https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/configuration/LybraConfigurator.sol#L6-L12

Vulnerability details

In LybraConfigurator::getBadCollateralRatio we have the following code:

if(vaultBadCollateralRatio[pool] == 0) return vaultSafeCollateralRatio[pool] - 1e19;
return vaultBadCollateralRatio[pool];

If vaultSafeCollateralRatio[pool] == 0 (i.e. vaultSafeCollateralRatio[pool] is not set) and vaultBadCollateralRatio[pool] is not yet set, the expression vaultSafeCollateralRatio[pool] - 1e19 will underflow.

Impact

It will make impossible to call liquidation in PEUSD Lybra vaults (LybraRETHVault, LybraWBETHVault and LybraWstETHVault) until vaultBadCollateralRatio[pool] is set. However, the setBadCollateralRatio function, which is used to set that mapping entry has the modifier onlyRole(DAO) and from comments in LybraConfigurator.sol:

DAO: A time-locked contract initiated by esLBR voting, with a minimum effective period of 14 days. After the vote is passed, only the developer can execute the action.

, which means that it will be necessary to wait at least 14 days in order to recover from this situation. Alternatively, vaultSafeCollateralRatio[pool] may be set, but setSafeCollateralRatio function, which is used for that, may be only called by TIMELOCK, and from another comment in LybraConfigurator.sol:

TIMELOCK: A time-locked contract controlled by the developer, with a minimum effective period of 2 days.

, which means that it will still be necessary to wait at least two days in order to fix the issue.

From Code4Rena docs:

2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.

Here, the availability is impacted and value leaked - lack of the possibility to liquidate "unhealthy" loans makes it possible for PEUSD to depeg. There are also external requirements:

Proof of Concept

Please modify getAssetPrice function and insert one extra function at the end to LybraWBETHVault.sol as follows:

function getAssetPrice() public override returns (uint256) {
        //return (_etherPrice() * IWBETH(address(collateralAsset)).exchangeRate()) / 1e18;
        return assetPrice;
    }

    uint public assetPrice;

    function setAssetPrice(uint newPrice) public
    {
        assetPrice = newPrice;
    }

It will make possible to simulate changes in ETH price. Please also run the following hardhat test:

const {ethers} = require("hardhat");
const {Contract} = require("ethers");
const {Interface} = require("ethers/lib/utils");
const {mine} = require("@nomicfoundation/hardhat-network-helpers");
const { assert, expect } = require("chai");

function toEther(quantity)
{
  return ethers.utils.parseEther(quantity);
}

function format(quantity)
{
  return ethers.utils.formatEther(quantity);
}

async function setUpConfigVault(Configurator, Vault) {
    await Configurator.setMintVault(Vault.address, true);
    await Configurator.setMintVaultMaxSupply(Vault.address, toEther("10000000000"));
    await Configurator.setBorrowApy(Vault.address, 200);

    //await Configurator.setSafeCollateralRatio(Vault.address, toEther("160"));
    //await Configurator.setBadCollateralRatio(Vault.address, toEther("150"));
}

async function deploy() {
  const goerliEndPoint = '0xbfD2135BFfbb0B5378b56643c2Df8a87552Bfa23'
  const goerliChainId = 10121

  /// INTERFACES

  const IStETH = new Interface([
    "function submit(address) external payable returns (uint256)"
  ]);

  const IWBETH = new Interface([
    "function exchangeRate() external view returns (uint256)",
    "function deposit(address) external payable",
    "function balanceOf(address) public view returns (uint256)"
  ]);

  const IWstETH = new Interface([
    "function stEthPerToken() external view returns (uint256)",
    "function wrap(uint256) external returns (uint256)",
    "function balanceOf(address) public view returns (uint256)"
  ]);

  const IRETH = new Interface([
    "function getExchangeRate() external view returns (uint256)",
    "function balanceOf(address) public view returns (uint256)"
  ]);

  /// ADRESSES
  STETH_ADDRESS = "0xae7ab96520DE3A18E5e111B5EaAb095312D7fE84";
  WBETH_ADDRESS = "0xa2E3356610840701BDf5611a53974510Ae27E2e1";
  WSTETH_ADDRESS = "0x7f39C581F595B53c5cb19bD0b3f8dA6c935E2Ca0";
  RETH_ADDRESS = "0xae78736Cd615f374D3085123A210448E74Fc6393";
  RK_POOL_ADDRESS = "0xDD3f50F8A6CafbE9b31a427582963f465E745AF8";

  accounts = await ethers.getSigners()
  owner = accounts[0].address
  Alice = accounts[1];
  Bob = accounts[2];
  Charlie = accounts[3]

  /// ON CHAIN CONTRACTS
  StETH = new Contract(STETH_ADDRESS, IStETH, ethers.provider);
  WBETH = new Contract(WBETH_ADDRESS, IWBETH, ethers.provider);
  WSTETH = new Contract(WSTETH_ADDRESS, IWstETH, ethers.provider);
  RETH = new Contract(RETH_ADDRESS, IRETH, ethers.provider);

  /// FACTORIES
  const OracleFactory = await ethers.getContractFactory("mockChainlink")
  const EUSDFactory = await ethers.getContractFactory("EUSD")
  const ConfiguratorFactory = await ethers.getContractFactory("Configurator")
  const LybraStETHDepositVaultFactory = await ethers.getContractFactory("LybraStETHDepositVault")
  const GovernanceTimelockFactory = await ethers.getContractFactory("GovernanceTimelock")
  const EUSDMiningIncentivesFactory = await ethers.getContractFactory("EUSDMiningIncentives")
  const esLBRBoostFactory = await ethers.getContractFactory("esLBRBoost")
  const LBRFactory = await ethers.getContractFactory("LBR")
  const esLBRFactory = await ethers.getContractFactory("esLBR")
  const PeUSDMainnetFactory = await ethers.getContractFactory("PeUSDMainnet")
  const ProtocolRewardsPoolFactory = await ethers.getContractFactory("ProtocolRewardsPool")
  const mockCurveFactory = await ethers.getContractFactory("mockCurve")
  const mockUSDCFactory = await ethers.getContractFactory("mockUSDC")
  const mockLBRPriceOracleFactory = await ethers.getContractFactory("mockLBRPriceOracle")
  const LybraWBETHVaultFactory = await ethers.getContractFactory("LybraWBETHVault");
  const LybraWstETHVaultFactory = await ethers.getContractFactory("LybraWstETHVault");
  const LybraRETHVaultFactory = await ethers.getContractFactory("LybraRETHVault");

  /// DEPLOYMENTS
  const Oracle = await OracleFactory.deploy()
  const mockLBRPriceOracle = await mockLBRPriceOracleFactory.deploy()
  const GovernanceTimelock = await GovernanceTimelockFactory.deploy(1,[owner],[owner],owner);
  const esLBRBoost = await esLBRBoostFactory.deploy()
  const mockUSDC = await mockUSDCFactory.deploy()
  const mockCurvePool = await mockCurveFactory.deploy()
  Configurator = await ConfiguratorFactory.deploy(GovernanceTimelock.address, mockCurvePool.address)
  const LBR = await LBRFactory.deploy(Configurator.address, 8, goerliEndPoint)
  const esLBR = await esLBRFactory.deploy(Configurator.address)
  EUSD = await EUSDFactory.deploy(Configurator.address)
  //await Configurator.initEUSD(eusdMock.address)
  const EUSDMiningIncentives = await EUSDMiningIncentivesFactory.deploy(Configurator.address, esLBRBoost.address, Oracle.address, mockLBRPriceOracle.address)
  const ProtocolRewardsPool = await ProtocolRewardsPoolFactory.deploy(Configurator.address)
  PEUSD = await PeUSDMainnetFactory.deploy(Configurator.address, 8, goerliEndPoint)

  /// CONFIGURATIONS
  await mockCurvePool.setToken(EUSD.address, mockUSDC.address)

  await Configurator.setPremiumTradingEnabled(true);
  await Configurator.setEUSDMiningIncentives(EUSDMiningIncentives.address)
  await Configurator.initToken(EUSD.address, PEUSD.address)
  await EUSDMiningIncentives.setToken(LBR.address, esLBR.address)
  await ProtocolRewardsPool.setTokenAddress(esLBR.address, LBR.address, esLBRBoost.address);

  /// VAULT DEPLOYMENTS
  LybraWBETHVault = await LybraWBETHVaultFactory.deploy(PEUSD.address, Oracle.address, WBETH_ADDRESS, Configurator.address)
  StETHVault = await LybraStETHDepositVaultFactory.deploy(Configurator.address, STETH_ADDRESS, Oracle.address)
  LybraWstETHVault = await LybraWstETHVaultFactory.deploy(STETH_ADDRESS, PEUSD.address, Oracle.address, WSTETH_ADDRESS, Configurator.address);
  LybraRETHVault = await LybraRETHVaultFactory.deploy(PEUSD.address, Configurator.address, RETH_ADDRESS, Oracle.address, RK_POOL_ADDRESS);

  setUpConfigVault(Configurator, StETHVault).catch(err => console.error(err));;
  setUpConfigVault(Configurator, LybraWBETHVault).catch(err => console.error(err));;
  setUpConfigVault(Configurator, LybraWstETHVault).catch(err => console.error(err));
  setUpConfigVault(Configurator, LybraRETHVault).catch(err => console.error(err));
}

describe("Test", function () 
{
  before(async () => {
    const MAINNET_FORKING_URL = "<URL>";
    await ethers.provider.send("hardhat_reset", [{
      forking: { jsonRpcUrl: MAINNET_FORKING_URL, blockNumber: 17598029 }
    }]);
    await deploy();
  });

  it("2. `getBadCollateralRatio` bug", async function () {
    // helper function so that we can simulate ETH price changes
    await LybraWBETHVault.setAssetPrice(toEther("1600"));
    await LybraWBETHVault.connect(Alice).depositEtherToMint(toEther("1000"), {value: toEther("1.1")});
    await LybraWBETHVault.connect(Bob).depositEtherToMint(toEther("10000"), {value: toEther("11")});

    // simulate time lapse and ETH price drop, so that a call to `liquidate` is possible
    await mine(7200 * 365);
    await LybraWBETHVault.setAssetPrice(toEther("1000"));
    // Alices's loan is unhealthy now, since ETH price dropped by 60%
    await PEUSD.connect(Bob).approve(LybraWBETHVault.address, toEther("10000"));
    // but it's still impossible to liquidate - the next line will revert
    await LybraWBETHVault.connect(Bob).liquidation(Bob.address, Alice.address, toEther("0.5"));

  });
})

It's necessary to substitute <URL> with a correct RPC URL. The test will output:

Error: VM Exception while processing transaction: reverted with panic code 0x11 (Arithmetic operation underflowed or overflowed outside of an unchecked block)
    at Configurator.getBadCollateralRatio (contracts/lybra/configuration/LybraConfigurator.sol:339)
    at LybraWBETHVault.liquidation (contracts/lybra/pools/base/LybraPeUSDVaultBase.sol:128)

Tools Used

VS Code

Recommended Mitigation Steps

Change vaultSafeCollateralRatio[pool] to getSafeCollateralRatio(pool) in getBadCollateralRatio. This way, even if safe and bad collateral ratios aren't set, getter functions will still return some default values without underflowing.

Assessed type

Under/Overflow

c4-pre-sort commented 1 year ago

JeffCX marked the issue as duplicate of #182

c4-judge commented 1 year ago

0xean changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

0xean marked the issue as grade-a