Lybra Finance - Iurii3's results

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/governance/LybraGovernance.sol#L67-L67

Vulnerability details

Description

According to the LybraGovernance.sol contract lines 120-122 forVotes is stored in proposalData[proposalId].supportVotes[0], while againstVotes in proposalData[proposalId].supportVotes[1]

        forVotes =  proposalData[proposalId].supportVotes[0];
        againstVotes =  proposalData[proposalId].supportVotes[1];
        abstainVotes =  proposalData[proposalId].supportVotes[2];

Vote succeeded when the number of "for" votes is bigger then "against" votes, but _voteSucceeded() function returns true when opposite (against > for)

Impact

DAO voted for proposal would not be possible to execute and thus DAO would be effectevely not-usable.

Recommended Mitigation Steps

Change comparison in _voteSucceeded() function to

return proposalData[proposalId].supportVotes[0] > proposalData[proposalId].supportVotes[1]

Assessed type

Governance

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/governance/LybraGovernance.sol#L60

Vulnerability details

Description

Governance quorum reached is when the sum of votes bigger then quorum threshold. Currently, _quorumReached function returns true if

proposalData[proposalId].supportVotes[1] + proposalData[proposalId].supportVotes[2] >= quorum(proposalSnapshot(proposalId));

However, according to the LybraGovernance.sol contract lines 120-122 - proposalData[proposalId].supportVotes[1] is againstVotes, proposalData[proposalId].supportVotes[2] is abstainVotes

        forVotes =  proposalData[proposalId].supportVotes[0];
        againstVotes =  proposalData[proposalId].supportVotes[1];
        abstainVotes =  proposalData[proposalId].supportVotes[2];

And function _quorumReached will return True only when amount of against and abstrain votes bigger then threshold

Impact

Votes will unlikely get enough against/abstain votes to pass threshold and proposals won't be exetucted

Recommended Mitigation Steps

Use total amount of votes as a base for calculating _quorumReached

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

Assessed type

Governance

Lines of code

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

Vulnerability details

Impact

User borrowed amount and poolTotalPeUSDCirculation amount calculated incorrectly and will be lower then they should be.

User's debt has two parts:

• feeStored[user];
• borrowed[_onBehalfOf];

Executing _repay() function firstly cover feeStored[user] and then everything that is left should be deducted from the borrowed[_onBehalfOf]. The amount deducted from the borrowed[_onBehalfOf] should be calculated as amount less coveredFee, instead it just subtract the whole amount

borrowed[_onBehalfOf] -= amount;
poolTotalPeUSDCirculation -= amount;

Moreover, these changes should be made only if repay amount > totalFee as poolTotalPeUSDCirculation do not include feeStored

Recommended Mitigation Steps

These lines should be moved into the true branch of if(amount >= totalFee) statement

borrowed[_onBehalfOf] -= amount - totalFee;
poolTotalPeUSDCirculation -= amount - totalFee; 

Assessed type

Math

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/LybraRETHVault.sol#L46-L48 https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/LybraWbETHVault.sol#L34-L36

Vulnerability details

Impact

wbETH and rETH contract provides information about their price comparing to the ETH. This information is used by the Lybra Finance in getAssetPrice() function to calculate the actual price of the wbETH and rETH contracts may return the while depositing in vault, minting, calculating liquidation, withdrawing from pool.

Currently, the getAssetPrice() implemented incorrectly and will always revert making it impossible to use nearly every functionality of the project.

Correct name of the function to get the price is getExchangeRate() for rETH and exchangeRate() for wbETH.

Recommended Mitigation Steps

Change code in LybraRETHVault.solL46-48 to

    function getAssetPrice() public override returns (uint256) {
        return (_etherPrice() * IRETH(address(collateralAsset)).getExchangeRate()) / 1e18;
    }

Change code in LybraWbETHVault.solL34-36 to

    function getAssetPrice() public override returns (uint256) {
        return (_etherPrice() * IWBETH(address(collateralAsset)).exchangeRate()) / 1e18;
    }

Assessed type

Math

1. During initiation of tokens in LybraConfiguration.sol wrong interface is used

LybraConfigurator.solL100

Should use IPeUSD instead of IEUSD. However, this mistake won't escalate into bigger error as IEUSD interface has all necessary functions

Correct line of code: if (address(peUSD) == address(0)) peUSD = IPeUSD(_peusd);

2. setTokenMiner function may revert due to array length discrepancies

LybraConfigurator.solL235-240

setTokenMiner function has two arguments - array of _contracts and array of _bools. In case their length is not equal function will revert while using extensive amount of gas in a for loop. Adding require(_contracts.length == _bools.length, "Length of contracts not equal to bools"); will save gas in this situation

3. Check in setLockStatus might be done wrongly

esLBRBoost.solL41

Considering Lybra Fincance allow users to extend the time of lock-up, this require statement is not correctly implemented.

For example

• user's current LockStatus is (unlockTime = +7 days from now; duration = 90 days, miningBoost = 30 * 1e18)
• user want to change in to (unlockTime = +30 days from now; duration = 30 days, miningBoost = 20 * 1e18)
• his lock-up time will increase from 7 to 30 days, but it is currently not allowed as overall duration will be lowered.

Team should consider rewriting requirement to require(userStatus.unlockTime <= block.timestamp + _setting.duration, "Your lock-in period has not ended, and the term can only be extended, not reduced.");

4. Vaults would be almost impossible to use with assets with decimals < 18.

LybraEUSDVaultBase.solL73 LybraPeUSDVaultBase.solL59

Currently Lybra Vaults may be used with 18-decimal LSD assets - stETH, wstETH, rETH, wbETH.

As it stated in Lybra docs "Collateral is any asset that a borrower must provide to take out a loan, acting as security for the debt." Should there appear LSD asset with lower decimals, or if team decides to add 6-decimal USDC/USDT as a possible collateral this requirement require(assetAmount >= 1 ether, "") would be passed only with much bigger amount then anticipated. For 6 decimal assets user would need to deposit 1_000_000_000_000 token to pass this requirement

Team might consider adding mapping with assets decimals (address => uint) or move this requirement from base abstract contract to higher level contract for particular asset

5. Check before transfer would save gas cost in case of revert LybraEUSDVaultBase.solL109

6. Wrong notation. Should use collateral asset instead of stETH LybraPeUSDVaultBase.solL80 LybraPeUSDVaultBase.solL123 LybraPeUSDVaultBase.solL149

7. Unnecessary assignment LybraRETHVault.solL18

rkPool value is added during the deployment in constructor, no need to add it directly in the contract.

8. Wrong address in notation LybraWbETHVault.solL16 0xae78736Cd615f374D3085123A210448E74Fc6393 is rETH address, correct wbETH address is 0xa2E3356610840701BDf5611a53974510Ae27E2e1

esLBR.solL6 esLBR.solL20

uint256 maxSupply = 100_000_000 * 1e18; in line 20 while 55 mln in notation

EUSD.solL149 EUSD.solL196 EUSD.solL222 EUSD.solL246 EUSD.solL266 EUSD.solL330 EUSD.solL364 EUSD.solL389

functions has "the contract must not be paused" in notation but actually do not implement any check for pause

9. Use SafeMath for consistency EUSD.sol#L425 EUSD.sol#L444

EUSD.sol imports SafeMath library and uses it for most arithmetical operations. But in two occasions += and -= used. Consider changing to SafeMath .add/.sub for consistency