Lybra Finance - RedTiger's results

Lines of code

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

Vulnerability details

Impact

Stakers of the esLBR stablecoin are experiencing significant loss of rewards when attempting to claim their protocol earnings. This issue results in users not receiving the rewards they should normally obtain.

Proof of Concept

The distributeRewards() function in the LybraConfigurator contract is responsible for distributing rewards to the LybraProtocolRewardsPool based on the available balance of eUSD. Under certain conditions, the eUSD amount is converted to another stablecoin and distributed to esLBR stakers.

esLBR stakers can claim their rewards using the getReward() function in the ProtocolsRewardsPool.sol contract. However, after the eUSD amount is distributed to stakers, the calculation of the stablecoin amount is flawed: https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/miner/ProtocolRewardsPool.sol#L209

uint256 tokenAmount = (reward - eUSDShare) * token.decimals() / 1e18;

The token.decimals() function returns an integer value (6 for USDC/USDT, 18 for DAI/FRAX), while both eUSDShare and reward have 18 decimal places (similar to eUSD). To correctly convert (reward - eUSDShare) to match the decimal places of the token, the calculation should be:

((reward - eUSDShare) * 10 ** token.decimals()) / 1e18.

The current incorrect calculation results in tokenAmount being significantly smaller than expected, at least 166,666 times smaller when considering a factor of (10^6)/6.

Tools Used

Manual review

Recommended Mitigation Steps

To resolve this issue, it is recommended to use the following line for calculating the token amount instead:

((reward - eUSDShare) * 10 ** token.decimals()) / 1e18

This adjustment ensures that the tokenAmount accurately reflects the intended value.

Assessed type

Decimal

Lines of code

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

Vulnerability details

Impact

Stakers of esLBR face two possible issues related to reward calculation. They might receive an excessive amount of rewards or be unable to claim their rewards altogether.

Proof of Concept

The NotifyRewardAmount() function receives stablecoin tokens sent by the configurator contract and records the accumulation of protocol rewards per esLBR held. The calculation of rewards is correct when the reward is in eUSD. However, when the reward is in another stablecoin, the calculation is incorrect.

The problematic line is as follows: https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/miner/ProtocolRewardsPool.sol#L236

rewardPerTokenStored = rewardPerTokenStored + (amount * 1e36 / token.decimals()) / totalStaked();

The token.decimals() function returns an integer value (6 for USDC/USDT, 18 for DAI/FRAX), while both amount and rewardPerTokenStored have 18 decimal places (similar to eUSD). The code comments mention converting the token decimals to 18 for consistent calculations, but the mistake lies in dividing the amount by token.decimals() instead of 10**token.decimals().

Due to this error, the rewardPerTokenStored value becomes excessively large, at least 166,666 times larger than intended. Since rewardPerTokenStored is used for eUSD and the other stablecoin defined in the configurator, users claiming rewards will either be prevented from claiming due to insufficient tokens or receive disproportionately huge amounts of rewards. Consequently, the rewards pool will be drained.

Tools Used

Manual review

Recommended Mitigation Steps

To address this issue, it is recommended to use the following line for calculating the token amount:

rewardPerTokenStored = rewardPerTokenStored + (amount * 1e36 / (10**token.decimals())) / totalStaked();

This adjustment ensures accurate reward calculations and prevents excessive rewards or reward claiming issues.

Assessed type

Decimal

Lines of code

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

Vulnerability details

Impact

In the default configuration, liquidation is not possible for non-rebasing vaults. All liquidation calls will revert, hindering the correct functioning of the protocol and putting assets at risk. PEUSD can become undercollateralized.

Proof of Concept

The LybraPeUSDVaultBase.sol file contains the liquidation function with the following require statement at the beginning:

require(onBehalfOfCollateralRatio < configurator.getBadCollateralRatio(address(this)), "Borrowers collateral ratio should be below badCollateralRatio");

This requirement is intended to ensure that the position can be liquidated when its collateral ratio falls below the BadCollateralRatio. The getBadCollateralRatio function is called to obtain this value.

The issue lies in the fact that getBadCollateralRatio directly calls vaultSafeCollateralRatio[pool] instead of using getSafeCollateralRatio. https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/configuration/LybraConfigurator.sol#L338-L340

function getBadCollateralRatio(address pool) external view returns(uint256) {
      if(vaultBadCollateralRatio[pool] == 0) return vaultSafeCollateralRatio[pool] - 1e19;
      return vaultBadCollateralRatio[pool];

Consequently, if the SafeCollateralRatio is not defined, the function will revert because it is supposed to return a uint256 but ends up with a negative value (0 - 1e19).

As a result, the liquidation function will also revert. Liquidations for non-rebasing vaults become impossible, and PEUSD may lose value due to insufficient collateral to back it. This situation can only be resolved when governance sets the SafeCollateralRatio. However, the setSafeCollateralRatio function has a timelock of at least two days, causing delays in addressing the issue.

In the event of a drop in the price of ETH, numerous liquidations will not occur.

Tools Used

Tools Used

Manual review

Recommended Mitigation Steps

To mitigate this vulnerability, modify the getBadCollateralRatio function to use getSafeCollateralRatio(pool) instead of directly accessing vaultSafeCollateralRatio[pool]. This adjustment ensures that the function returns the correct value and allows liquidation to proceed as intended.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

rETH deposits are not working

Proof of Concept

The rETH vault calls getExchangeRatio() instead of getExchangeRate() in the getAssetPrice() https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/LybraRETHVault.sol#L47 ```solidity
return (_etherPrice() * IRETH(address(collateralAsset)).getExchangeRatio()) / 1e18;

Or getExchangeRatio() does not exist for the rETH token;
https://etherscan.io/token/0xae78736cd615f374d3085123a210448e74fc6393#readContract
As a result the depositEtherToMint which call the getAssetPrice will always revert. 
And no rETH deposits can happens.


## Tools Used
Manual Review
## Recommended Mitigations
Use getExchangeRate() instead of  getExchangeRatio()  


## Assessed type

Other

Lines of code

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

Vulnerability details

Impact

wbETH deposits are not working

Proof of Concept

The wbETH vault calls exchangeRatio() instead of getExchangeRate() in the getAssetPrice() ```solidity return (_etherPrice() * IWBETH(address(collateralAsset)).exchangeRate()) / 1e18;

Or exchangeRatio() does not exist for the wbETH token;
https://etherscan.io/token/0xa2E3356610840701BDf5611a53974510Ae27E2e1#readProxyContract 
As a result the depositEtherToMint which call the getAssetPrice will always revert. 
And no wbETH deposits can happen.

## Tools Used
Manual Review
## Recommended Mitigation Steps
Use ExchangeRate() instead of  ExchangeRatio()


## Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/5d70170f2c68dbd3f7b8c0c8fd6b0b2218784ea6/contracts/lybra/configuration/LybraConfigurator.sol#L126

Vulnerability details

Impact

The current implementation allows for a situation where the BadCollateralRatio can be greater than the SafeCollateralRatio. This vulnerability can result in instant liquidation of an EUSD minter's assets and subsequent loss of funds, even if their collateral ratio is above the Safe Collateral Ratio. This causes a loss of funds for the minter due to the penalty incurred on the liquidated deposited asset.

Proof of Concept

https://github.com/code-423n4/2023-06-lybra/blob/5d70170f2c68dbd3f7b8c0c8fd6b0b2218784ea6/contracts/lybra/configuration/LybraConfigurator.sol#L126C14-L126C35

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

The setBadCollateralRatio function is used by the DAO to define the BadCollateralRatio. However, the last condition in the require statement allows for a BadCollateralRatio greater than the SafeCollateralRatio. This condition should be reconsidered :

newRatio <= vaultSafeCollateralRatio[pool] + 1e19

Furthermore, the setSafeCollateralRatio function allows the DAO to set the SafeCollateralRatio to 140, with the only condition being that it must be 10% higher than the BadCollateralRatio. This works if the BadCollateralRatio is not yet defined or is at most 130. https://github.com/code-423n4/2023-06-lybra/blob/5d70170f2c68dbd3f7b8c0c8fd6b0b2218784ea6/contracts/lybra/configuration/LybraConfigurator.sol#L198

require(newRatio >= vaultBadCollateralRatio[pool] + 1e19, "PeUSD vault safe collateralRatio should more than bad collateralRatio");

If the DAO mistakenly or dishonestly sets the BadCollateralRatio for non-rebasing pools to 150, it will be implemented due to the current implementation's flaw. This scenario leads users to believe that their positions are safe when they are not, resulting in financial losses when they are instantly liquidated (MEV bots, etc.). BadCollateralRatio : 150 SafeCollateralRatio : 140

Tools Used

Manual Review

Recommended Mitigation Steps

Correct newRatio <= vaultSafeCollateralRatio[pool] + 1e19 to newRatio < getSafeCollateralRatio(pool)

https://github.com/code-423n4/2023-06-lybra/blob/5d70170f2c68dbd3f7b8c0c8fd6b0b2218784ea6/contracts/lybra/configuration/LybraConfigurator.sol#L127C42-L127C42

The Rebase vault is not impacted because the BadCollateralRatio is read from a variable inside the vault and not from the Configurator.

Assessed type

Math

Lines of code

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

Vulnerability details

Impact

For non-rebasing vaults, the SafeCollateralRatio can be set as low as 10 by the Governance or the deployer, which would make the EUSD unbacked, worth only 1/10 of its value. Users could borrow 10 times their collateral and redeem the EUSD for Stacked ETH, which has real value.

Proof of Concept

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

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

The getBadCollateralRatio function returns 130 if the BadCollateralRatio is not set. However, the setSafeCollateralRatio function does not use the getBadCollateralRatio function to retrieve the BadCollateralRatio. Instead, it directly calls vaultBadCollateralRatio[pool] for non-rebasing vaults.

By directly calling vaultBadCollateralRatio[pool], setSafeCollateralRatio will retrieve a value of 0 for the BadCollateralRatio if it is not set.

As a result, the requirement in setSafeCollateralRatio to have SafeCollateralRatio >= vaultBadCollateralRatio[pool] + 10 is valid for a SafeCollateralRatio of 10 when the BadCollateralRatio is not set.

require(newRatio >= vaultBadCollateralRatio[pool] + 1e19)

Since the BadCollateralRatio is supposed to have a default value of 130 for non-rebasing pools, it is possible that it is not set manually before setting the SafeCollateralRatio. A governance proposal could pass with a value of 10 for SafeCollateralRatio, leading to the draining of all available Staked ETH for redemption of non-rebasing pools.

Users can mint 10 times the value of their collateral in EUSD and then redeem their EUSD for real collateral. Furthermore, the value of EUSD would drop dramatically, resulting in financial losses for all EUSD holders.

Tools Used

Manual Review

Recommended Mitigation Steps

Use getBadCollateralRatio(pool) instead of vaultBadCollateralRatio[pool] when the value is needed for comparison. Similarly, use getSafeCollateralRatio(pool) instead of vaultSafeCollateral[pool]

Assessed type

Other

Low Risk and Non-Critical Issues

L01 - No part of the bounty is returned to the mining pool

According to Lybra blog post a percentage of the rewards bounty sold at a discount should be returned to the mining pool " If the qualifier drops below the minimum 5% threshold, the user will become ineligible for subsequent esLBR emissions. Simultaneously, a bounty equal to the amount of emissions that the user has earned while ineligible will be placed. This bounty can be purchased by any user at a 50% discount in LBR.

3. The LBR received will then be distributed as follows: 10% will be burned, and the remaining 90% will be returned to the mining pool."

However, we can see in the code that 100% of the bounty is burned. https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/miner/EUSDMiningIncentives.sol#L218

IesLBR(LBR).burn(msg.sender, biddingFee);

L02- Error in the comments of reStake() in ProtocolRewardsPool.sol

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

• @dev Convert unredeemed and converting ESLBR tokens back to LBR. Should be
• @dev Convert unredeemed and converting LBR tokens back to ESLBR

L03 Wrong address in the comments for WBETH

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/LybraWbETHVault.sol#L16 The address of the WBETH contract listed in the comments for WBETH is 0xae78736Cd615f374D3085123A210448E74Fc6393. But this address is the goerli RETH address. This comment should be change to the WBETH goerli address. Or mainet WBETH address.

L04 Allowance check could be improved

lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L131 The allowance check in the liquidiation function checks only that the allowance is greater than zero. A more complete check should be that EUSD.allowance(provider, address(this)) > assetAmount.

 require(EUSD.allowance(provider, address(this)) > 0, "provider should authorize to provide liquidation EUSD");

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

L05 stETH mentionned 3 times in LybraPeUSDVaultBase.sol

LybraPeUSDVaultBase.sol is about non-rebasing LST. So it should not mention stETH. But 3 times in the comments of this file stETH is mentionned. https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L80

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

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

L06 esLBR total supply is not clear

The maxSupply of esLBR is defined as 100 000 000 tokens here https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/token/esLBR.sol#L20

uint256 maxSupply = 100_000_000 * 1e18;

But the comments at the beginning of the file mention a limit of 55 millions tokens in the esLBRMinter, and it is the only way to get esLBR

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

//The maximum amount that can be minted through the esLBRMinter contract is 55 million.

L07 Incosistency between Price of Oracle used

The app uses the liquity oracle to get the price of ETHER, but also sometimes uses the chainlink ETH/USD feed directly. These are not the same as the Liquity Oracle uses chainlink but also TELLOR as a backup and contains additional checks. Liquity Oracle https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L242 Chainlink Oracle : https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/miner/EUSDMiningIncentives.sol#L63

L08 - executeFlashloan doest not check if there is enough PEUSD in the contract to lend

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/token/PeUSDMainnetStableVision.sol#L129 executeFlashloan does not check if there is enough PEUSD in the contract to lend.

N01- No need for LBR to be an OFT token

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

contract LBR is BaseOFTV2, ERC20 {

It is not mentionned in any documentations that LBR should be an OFT token. So LBR should not be BaseOFTV2.

N02 - Stuck ETHER due to Unecessary Payable functions

In PeUSDMainnetStableVision.sol , convertToPeUSDAndCrossChain and executeFlashloan are payable, but they don't manipulate ETHER. They should not be payable. As a result Ether can be stuck in the contract since there is no way to distribue this ETHER is sent by error. Eiher remove Payable when not needed or use " address(this).balance" to get the amount of Ether in the contract before depositing to LST to avoid Ether being stuck. https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/token/PeUSDMainnetStableVision.sol#L102 https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/token/PeUSDMainnetStableVision.sol#L129