Lybra Finance - Vagner's results

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L125-L146 https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraEUSDVaultBase.sol#L154-L176 https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraEUSDVaultBase.sol#L187-L211

Vulnerability details

Impact

The liquidation functions in both contracts need to take an address of a provider, from which it will be taken the eusdAmount or peusdAmount after it verifies that the provider allowance to the contract is greater than 0. Basically before calling the liquidation function, the provider needs to approve the contract some amount of tokens so the function _repay can transferFrom the amount of borrowed tokens https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L192-L210 After that the liquidation functions checks if provider is the same as msg.sender and transfer the assets to the provider if it is the same as msg.sender or it will give a small amount to msg.sender and the rest to provider https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L138-L144 The problem relies in the fact that anybody can front-run the liquidation function getting a small amount of rewards, without risking or losing anything beside the higher gas fee paid.

Proof of Concept

Take this simple case :

• Alice sees an opportunity to liquidate someone and get the assets for herself so she first approve the contract with the amount of EUSD or PeUSD necessary
• Bob sees the same opportunity as Alice but doesn't have the tokens to buy the assets for himself so he waits for Alice to approve the amount of tokens to the contract and then front-run her transaction with his transaction setting the provider address to Alice one
• All the passes will check and Bob will get a small or big reward, depending if it was a normal or super liquidation, taking a share of Alice rewards, without losing or risking anything, since Alice paid for everything
• This process can be repeated as many time as possible getting asset rewards for almost free

Tools Used

Manual review

Recommended Mitigation Steps

Do more checks in the liquidation function or change how the function work so it is not that easily front-runnable

Assessed type

Timing

Lines of code

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

Vulnerability details

Impact

The function _repay is used to repay the borrowed PeUSD in the cases of liquidation or simple repayment. The function checks if the borrowed amount of the user + totalFee is greater or equal to the _amount wanted to be repayed. If that's not the case the amount would be equal to borrowed + totalFee as can be seen here https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L196 The problem relies in the fact that after it will transfer the totalFee to the configurator and burn the difference between the amount and totalFee, it will subtract from the borrowed[_onBehalfOf] the amount value https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L206 but in the case where the amount it will be equal to borrowed[_onBehalfOf] + totalFee than the amount will always be greater than borrowed[_onBehalfOf] which will result in revert 100% of the time because of the underflow.

Proof of Concept

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

Tools Used

Manual review

Recommended Mitigation Steps

Change the logic of the function so the subtraction would not revert in the case where amount is equal to borrowed[_onBehalfOf] + totalFee

Assessed type

Under/Overflow

Lines of code

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

Vulnerability details

Impact

The function getAssetPrice is used multiple times to calculate different values, but in the cases of LybraRETHVault.sol and LybraWbETHVault.sol this function would revert because in both contracts it calls the wrong function in the BETH and RETH deployed contracts.

Proof of Concept

The function getAssetPrice calculate the price of the collateral asset multiplying the price from the Oracle in _etherPrice function with the value from the getExchangeRatio for LybraRETHVault.sol https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/LybraRETHVault.sol#L47 and exchangeRatio for LybraWbETHVault.sol https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/LybraWbETHVault.sol#L35 but the functions on the RETH and BETH contract are not named getExchangeRatio and exchangeRatio but getExchangeRate and exchangeRate as can be seen on etherscan https://etherscan.io/address/0xae78736Cd615f374D3085123A210448E74Fc6393#code#F1#L92 https://etherscan.io/address/0x523177fbe442afb70b401d06bb11ec7b8684ecee#code#F21#L256

Tools Used

Manual review

Recommended Mitigation Steps

Change the name of the function to the ones on the deployed contracts so the getAssetPrice for those two cases would not revert all the time

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

The liquidation process in LybraPeUSDVaultBase.sol uses getBadCollateralRatio to see if the borrwers collateral ratio is less than vaultBadCollateralRatio https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L128 calling the getBadCollateralRatio function in LybraConfigurator.sol. The function checks if the vaultBadCollateralRatio variable is 0 and if that's the case it get the value from the subtraction of vaultSafeCollateralRatio and 1e19 https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/configuration/LybraConfigurator.sol#L339 but as we can see above vaultSafeCollateralRatio can also be uninitialized which in the case of getSafeCollateralRatiofunction would return 160 * 1e18 https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/configuration/LybraConfigurator.sol#L334 but for getBadCollateralRatio this would underflow and revert since the value of getSafeCollateralRatio would be 0.

Proof of Concept

If a person can be liquidated but the values of the 2 variables are not yet initialized, since they can be set just by the Timelock or the DAO, the function would revert all the time because of the subtraction.

Tools Used

Manual review

Recommended Mitigation Steps

Instead of using the variable vaultSafeCollateralRatio to do the subtraction in getBadCollateralRatio use the function getSafeCollateralRatio which would return for sure a value greater than 1e19, making the function not revert.

Assessed type

Under/Overflow