Lybra Finance - Co0nan's results

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/EUSD.sol#L2990-L304

Vulnerability details

Impact

The first depositor can be front run by an attacker and as a result will lose a considerable part of the assets provided.

Proof of Concept

The reward pool calculates the amount of shares to be minted upon deposit to every user via the EUSD.getSharesByMintedEUSD() function: https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/EUSD.sol#L2990-L304

function getSharesByMintedEUSD(uint256 _EUSDAmount) public view returns (uint256) {
        uint256 totalMintedEUSD = _totalSupply;
        if (totalMintedEUSD == 0) {
            return 0;
        } else {
            return _EUSDAmount.mul(_totalShares).div(totalMintedEUSD);
        }
    }

This function invoked when we call EUSD::mint https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/EUSD.sol#L411

function mint(address _recipient, uint256 _mintAmount) external onlyMintVault MintPaused returns (uint256 newTotalShares) {
        require(_recipient != address(0), "MINT_TO_THE_ZERO_ADDRESS");

        uint256 sharesAmount = getSharesByMintedEUSD(_mintAmount);
        if (sharesAmount == 0) {
            //EUSD totalSupply is 0: assume that shares correspond to EUSD 1-to-1
            sharesAmount = _mintAmount;
        }

        newTotalShares = _totalShares.add(sharesAmount);
        _totalShares = newTotalShares;

        shares[_recipient] = shares[_recipient].add(sharesAmount);

        _totalSupply += _mintAmount;

        emit Transfer(address(0), _recipient, _mintAmount);
    }

When the pool has no share supply, the amount of shares to be minted is equal to the assets provided. An attacker can abuse of this situation and profit of the rounding down operation when calculating the amount of shares if the supply is non-zero. This attack is enabled by the following components: frontrunning, rounding down the amount of shares calculated and regular ERC20 transfers.

Tools Used

Manual

Recommended Mitigation Steps

Consider requiring a minimal amount of share tokens to be minted for the first minter.

Assessed type

ERC4626

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/ProtocolRewardsPool.sol#L73

Vulnerability details

Impact

Minters can unstake their funds at any moment bypassing the lock time mechanism.

Proof of Concept

When miner try to unstake from the ProtocolRewardsPool, the function first check that the current timestamp > the unlock time.

function unstake(uint256 amount) external {
        require(block.timestamp >= esLBRBoost.getUnlockTime(msg.sender), "Your lock-in period has not ended. You can't convert your esLBR now.");
        esLBR.burn(msg.sender, amount);
        withdraw(msg.sender);
        uint256 total = amount;
        if (time2fullRedemption[msg.sender] > block.timestamp) {
            total += unstakeRatio[msg.sender] * (time2fullRedemption[msg.sender] - block.timestamp);
        }
        unstakeRatio[msg.sender] = total / exitCycle;
        time2fullRedemption[msg.sender] = block.timestamp + exitCycle;
        emit UnstakeLBR(msg.sender, amount, block.timestamp);
    }

The function getUnlockTime retrieve the user unlockTime from the userLockStatus mapping.

The user object of the mapping userLockStatus got set/updated through setLockStatus.

The issue here is that this function never get called across the code, as per the design, this function should be called through stake function to updates the user settings so he can't call withdraw unless his lock time passed.

Foundry test

1. Before running this test, please comment the following lines: https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/LBR.sol#L26 https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/LBR.sol#L33 https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/esLBR.sol#L31 https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/esLBR.sol#L33 https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/esLBR.sol#L39 https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/esLBR.sol#L40

2. Create test.t.sol file and put it into /lybra/contracts/

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";
import {LybraProxy} from "./lybra/Proxy/LybraProxy.sol";
import {LybraProxyAdmin} from "./lybra/Proxy/LybraProxyAdmin.sol";
// import {AdminTimelock} from "./lybra/governance/AdminTimelock.sol";
import {GovernanceTimelock} from "./lybra/governance/GovernanceTimelock.sol";
// import {LybraWBETHVault} from "./lybra/pools/LybraWbETHVault.sol";
import {esLBR} from "./lybra/token/esLBR.sol";
import {LybraWstETHVault} from "./lybra/pools/LybraWstETHVault.sol";
// import {LybraRETHVault} from "./lybra/pools/LybraRETHVault.sol";
// import {PeUSD} from "./lybra/token/PeUSD.sol";
import {esLBRBoost} from "./lybra/miner/esLBRBoost.sol";
import {LBR} from "./lybra/token/LBR.sol";
import {LybraStETHDepositVault} from "./lybra/pools/LybraStETHVault.sol";
// import {StakingRewardsV2} from "./lybra/miner/stakerewardV2pool.sol";
// import {LybraGovernance} from "./lybra/governance/LybraGovernance.sol";
import {PeUSDMainnet} from "./lybra/token/PeUSDMainnetStableVision.sol";
import {ProtocolRewardsPool} from "./lybra/miner/ProtocolRewardsPool.sol";
// import {EUSD} from "./lybra/token/EUSD.sol";
import {Configurator} from "./lybra/configuration/LybraConfigurator.sol";
import {EUSDMiningIncentives} from "./lybra/miner/EUSDMiningIncentives.sol";
// import {LybraEUSDVaultBase} from "./lybra/pools/base/LybraEUSDVaultBase.sol";
// import {LybraPeUSDVaultBase} from "./lybra/pools/base/LybraPeUSDVaultBase.sol";
import {mockChainlink} from "./mocks/chainLinkMock.sol";
import {stETHMock} from "./mocks/stETHMock.sol";
import {EUSDMock} from "./mocks/mockEUSD.sol";
import {mockCurve} from "./mocks/mockCurve.sol";
import {mockUSDC} from "./mocks/mockUSDC.sol";
import {mockLBRPriceOracle} from "./mocks/mockLBRPriceOracle.sol";

/* remappings used
./lybra=contracts/lybra/
./mocks=contracts/mocks/
 */
contract CounterScript is Test {
    address goerliEndPoint = 0xbfD2135BFfbb0B5378b56643c2Df8a87552Bfa23;

    LybraProxy proxy;
    LybraProxyAdmin admin;
    // AdminTimelock timeLock;
    GovernanceTimelock govTimeLock;
    // LybraWbETHVault wbETHVault;
    // esLBR lbr;
    // LybraWstETHVault stETHVault;
    mockChainlink oracle;
    mockLBRPriceOracle lbrOracleMock;
    stETHMock stETH;
    esLBRBoost eslbrBoost;
    mockUSDC usdc;
    mockCurve curve;
    Configurator configurator;
    LBR lbr;
    esLBR eslbr;
    EUSDMock usd;
    EUSDMiningIncentives eusdMiningIncentives;
    ProtocolRewardsPool rewardsPool;
    LybraStETHDepositVault stETHVault;
    PeUSDMainnet peUsdMainnet;
    address owner = address(7);
    // admins && executers of GovernanceTimelock
    address[] govTimelockArr;

    function setUp() public {
        vm.startPrank(owner);
        oracle = new mockChainlink();
        lbrOracleMock = new mockLBRPriceOracle();
        stETH = new stETHMock();
        govTimelockArr.push(owner);
        govTimeLock = new GovernanceTimelock(
            1,
            govTimelockArr,
            govTimelockArr,
            owner
        );
        eslbrBoost = new esLBRBoost();
        usdc = new mockUSDC();
        curve = new mockCurve();
        //  _dao , _curvePool
        configurator = new Configurator(address(govTimeLock), address(curve));
        // _config , _sharedDecimals , _lzEndpoint
        lbr = new LBR(address(configurator), 8, goerliEndPoint);
        // _config
        eslbr = new esLBR(address(configurator));
        // _config
        usd = new EUSDMock(address(configurator));
        // _config, _boost, _etherOracle, _lbrOracle
        eusdMiningIncentives = new EUSDMiningIncentives(
            address(configurator),
            address(eslbrBoost),
            address(oracle),
            address(lbrOracleMock)
        );
        // _config
        rewardsPool = new ProtocolRewardsPool(address(configurator));
        // _config, _stETH, _oracle
        stETHVault = new LybraStETHDepositVault(
            address(configurator),
            address(stETH),
            address(oracle)
        );
        // _config, _sharedDecimals, _lzEndpoint
        peUsdMainnet = new PeUSDMainnet(
            address(configurator),
            8,
            goerliEndPoint
        );

        configurator.initToken(address(usd), address(peUsdMainnet));
        curve.setToken(address(usd), address(usdc));
        configurator.setMintVault(address(stETHVault), true);
        configurator.setPremiumTradingEnabled(true);
        configurator.setMintVaultMaxSupply(
            address(stETHVault),
            10_000_000_000 ether
        );
        configurator.setBorrowApy(address(stETHVault), 200);
        configurator.setEUSDMiningIncentives(address(eusdMiningIncentives));
        eusdMiningIncentives.setToken(address(lbr), address(eslbr));
        rewardsPool.setTokenAddress(
            address(eslbr),
            address(lbr),
            address(eslbrBoost)
        );

        // Missing configurator.initEUSD(this.EUSDMock.address) as initEUSD in configurator does not exist.
        // And it's not same as initToken. 
        vm.stopPrank();
    }

    function test_deposit() public {
        address alice = address(1234);
        stETH._mintShares(alice,100e18);
        //console.log(stETH.balanceOf(alice));

        // mint 100 token for Alice
        lbr.mint(alice, 100e18);

        // Call ProtocolRewardsPool.stake(10)
        vm.startPrank(alice);

        rewardsPool.stake(10e18);
        console.log("EsLBR balance of Alice after stacking");
        console.log(eslbr.balanceOf(alice));

        console.log("Alice call unstake..");
        rewardsPool.unstake(10e18);
        console.log("EsLBR balance of Alice after un-stacking");
        console.log(eslbr.balanceOf(alice));

        vm.stopPrank();


    }
    
}

3. The function test_deposit shows that we can stake and unstake at any time since getUnlockTime never got set

Tools Used

Manual

Recommended Mitigation Steps

ProtocolRewardsPool::stake function should parse the stake option and call setLockStatus after confirming that this value is never set before, if its already set continue staking without calling setLockStatus

Assessed type

Context

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L134

Vulnerability details

Description

The function LybraPeUSDVaultBase::_repay calculate the fees required to be paid by the user and then check if the amount >= totalFee or not.

During repayment process, user have to pay the borrowed amount + totalFees. If the amount > totalFee, the function send the fee amount from the provider and burn the difference from the provider balance.

When the totalFee > amount which means the amount sent will not cover the fees, the function reach the else statement where it set the new fees to the totalFee - amount and send the amount to the configurator address.

Finally, the function reduce the total borrowed amount by amount value.

The issues with this flow are:

1. Case1 : amount >= totalFees.

The amount of tokens that will be burned calculated based on the difference between the amount and the totalFees, however, the function reduce the total borrowed amount based on the amount value itself. Hence, user borrowed amount will be reduced by higher value than it should. in some cases, user can have his borrowed amount reduced to Zero without having any token burned.

2. Case2 : totalFees > amount (else)

Technically, when the function reach the else statement we shouldn't reduce the borrowed amount since all the amount send has been paid to cover the fees, but the function still reduce the total amount so the user borrowed amount will be reduced even he didn't burn tokens.

Impact

User total borrowed amount will be reduced without burning the tokens. User will end up having PeUSD tokens minted without having his collateral at risk since his total borrowed amount is Zero.

Proof of Concept

Assume the following:

_amount argument = 50 borrowed amount = 50 totalFee = 70

1. At L196 the calculation of the amount will be:

uint256 amount = borrowed[_onBehalfOf] + totalFee >= _amount ? _amount : borrowed[_onBehalfOf] + totalFee;

amount = 50 + 70 >= 50 -> True
amount = _amount = 50

2. At L197 the if condition result in false, so we reach the else statement. This means the amount will only cover part of the fees.

3. At L202 the new feeStored calculated as follows:

feeStored[_onBehalfOf] = totalFee - amount;
70 - 50 = 20

4. The function update the feeStored to 20, at L203 the function send the amount to the configurator as the fees.

5. Even so the amount only cover part of the fees, the function reduce the borrowed amount at L206

borrowed[_onBehalfOf] -= amount;
50 - 50 = 0

6. Hence, Instead of paying 120 (borrowed amount + fees) borrower paid only 50 as fees without burning any token, and he skipped from paying 20 the remaining fee amount. The user will end up having 50 PeUSD in his balance + his original collateral.

Tools Used

Manual

Recommended Mitigation Steps

The function should reduce the total borrowed amount only inside the first if condition based on the difference between amount and totalFees

if(amount >= totalFee) {
            feeStored[_onBehalfOf] = 0;
            PeUSD.transferFrom(_provider, address(configurator), totalFee);
            PeUSD.burn(_provider, amount - totalFee);
            borrowed[_onBehalfOf] -= (amount - totalFee);
            poolTotalPeUSDCirculation -= (amount - totalFee);

....
....
....
-- borrowed[_onBehalfOf] -= amount;
-- poolTotalPeUSDCirculation -= amount;

Assessed type

Context

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/pools/LybraRETHVault.sol#L47

Vulnerability details

Impact

Deposits functionality on LybraRETHVault will be in DoS state due to calling undefined function on the rkPool when invoking getAssetPrice().

Proof of Concept

On LybraRETHVault, the function getAssetPrice() was supposed to get the value of rETH in terms of ETH. However, the function make an external call invoking getExchangeRatio, there is no such function defined on the rkPool matching this function. Instead the correct function is getExchangeRate

Tools Used

Manual

Recommended Mitigation Steps

Correct functions is:

  return (_etherPrice() * IRETH(address(collateralAsset)).getExchangeRate()) / 1e18;

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/pools/LybraWbETHVault.sol#L35

Vulnerability details

Impact

Deposits functionality on LybraWbETHVault will be in DoS state due to calling undefined function on the WBETH when invoking getAssetPrice().

Proof of Concept

On LybraWbETHVault, the function getAssetPrice() was supposed to get the value of WbETH in terms of ETH. However, the function make an external call invoking exchangeRatio, there is no such function defined on the WbETH Contract matching this function. Instead the correct function is exchangeRate

Tools Used

Manual

Recommended Mitigation Steps

Correct functions is:

return (_etherPrice() * IWBETH(address(collateralAsset)).exchangeRate()) / 1e18;

Assessed type

DoS

1. setPools will reset the pools array if it's called with an empty array. https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/EUSDMiningIncentives.sol#L93

 function setPools(address[] memory _pools) external onlyOwner {
        for (uint i = 0; i < _pools.length; i++) {
            require(configurator.mintVault(_pools[i]), "NOT_VAULT");
        }
        //@audit-qa if the function called with 0 array length. it will resetr the pools. Must check it's > 0
        pools = _pools;
    }

2. User can front-run the TX which invoke notifyRewardAmount and stake large amount to profit from the reward. https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/ProtocolRewardsPool.sol#L227

function notifyRewardAmount(uint amount, uint tokenType) external { // @audit-qa bot can fornt-run and stake large amount to before ditrbuiteRewards
        require(msg.sender == address(configurator));
        if (totalStaked() == 0) return;
        require(amount > 0, "amount = 0");
        if(tokenType == 0) {
            uint256 share = IEUSD(configurator.getEUSDAddress()).getSharesByMintedEUSD(amount);
            rewardPerTokenStored = rewardPerTokenStored + (share * 1e18) / totalStaked();
        } else if(tokenType == 1) {
            ERC20 token = ERC20(configurator.stableToken());
            rewardPerTokenStored = rewardPerTokenStored + (amount * 1e36 / token.decimals()) / totalStaked();
        } else {
            rewardPerTokenStored = rewardPerTokenStored + (amount * 1e18) / totalStaked();
        }
    }

3. executeFlashloan shouldn't be marked as payable, any ETH sent by mistake could be lost. https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/PeUSDMainnetStableVision.sol#L129

4. Due to rounding error on grabEsLBR function when calculating the amount to be burned, an attacker can send 3 WEI as amount which will result attacker mint 3 WEI without burning any tokens due to the calculation round down to Zero https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/ProtocolRewardsPool.sol#L131

function grabEsLBR(uint256 amount) external {
        require(amount > 0, "QMG");
        grabableAmount -= amount;
        LBR.burn(msg.sender, (amount * grabFeeRatio) / 10000);
        // 3 * 3000 / 10000 = 9000 / 10000 = 0
        esLBR.mint(msg.sender, amount);
    }

5. Wrong address for WBETH on LybraWBETHVault https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/pools/LybraWbETHVault.sol#L16

The commented mentioned that the address of WBETH is 0xae78736Cd615f374D3085123A210448E74Fc6393 but this is the address for the rETH contract used in LybraRETHVault. The correct address is 0xa2E3356610840701BDf5611a53974510Ae27E2e1. While this is not directly making an issue since it's just a comment, this can influence the deployer to use wrong address.