Lybra Finance - kutugu's results

Lines of code

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

Vulnerability details

Impact

EUSD treats sharesAmount == 0 as totalSupply == 0, which is wrong and when exchangeRate = share / amount > 1, resulting in excess tokens being minted to users.

Proof of Concept

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

Assume exchangeRate increases to 2.1:

1. User collateral value $10, want to mint 2 EUSD value $2
2. getSharesByMintedEUSD(2) = 0, so sharesAmount = 2 = 4 EUSD
3. The collateral price decreases to $4 and the user is liquidated

But the user is unaware, because he thinks that only 2 EUSD are mint

Tools Used

Manual review

Recommended Mitigation Steps

Check _totalSupply not sharesAmount

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

ProtocolRewardsPool rewards are divided into categories that cannot be simply added, mixing them together and impact on the previous staking users' reward.

Proof of Concept

We can look at how rewards are calculated based on the code. There are three categories:

• tokenType = 0, that is EUSD. The reward is converted to EUSD share for distribution
• tokenType == 1, that is USDC stablecoin, the reward is the amount of USDC
• Other, that is PEUSD, the reward is the amount of PEUSD

They are three different categories, can not be simply added together, we use EUSD share as the base:

• For USDC, the correct approach is to convert the USDC amount to EUSD amount, and then to EUSD share
• For PEUSD, a PEUSD may have 1.x EUSD, which should also be converted into EUSD amount, and then converted into EUSD shares

These is the problems of rewardPerTokenStored caculation in notifyRewardAmount, and there are corresponding problems in getReward: After fixing the calculation issue with rewardPerTokenStored, the reward(EUSD share) will need to be converted to the corresponding token amount when distributing USDC and PEUSD.

Finally, explain why there is an impact on the previous staking users' reward:

• Assume that the first reward distributed through EUSD, the staking users haven't claim rewards
• The second reward distributed through USDC, assuming that 1 EUSD = 1 USDC, but because it is not converted into share, the reward amount allocated for the second time is larger than that for the first time, diluting the reward of the first staking. At this time, if the user who stakes at the second time withdraws the reward, EUSD share will be withdrawn at 1:1, resulting in the loss of the reward for the user who stakes at the first time
• The same is true for PEUSD

Tools Used

Manual review

Recommended Mitigation Steps

The number of rewards should not be simply added up; they need to be converted to the same type first

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

ProtocolRewardsPool getReward function EUSD.transferShares's amount miscalculation, as a result, users cannot claim rewards when the contract does not have enough EUSD.

Proof of Concept

    uint256 balance = EUSD.sharesOf(address(this));
    uint256 eUSDShare = balance >= reward ? reward : reward - balance;
    EUSD.transferShares(msg.sender, eUSDShare);

If balance < reward, eUSDShare should be balance instead of reward - balance.

Tools Used

Manual review

Recommended Mitigation Steps

Use correct amount

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

The getExchangeRate / exchangeRate function is inexistence, causing LybraRETHVault / LybraWbETHVault vault to be unavailable.

Proof of Concept

According to the actual contract address, the names of the functions that obtain exchangeRate are getExchangeRate and exchangeRate, not getExchangeRatio or exchangeRatio.

Tools Used

Manual review

Recommended Mitigation Steps

Modify function name and interface

Assessed type

Context

Findings Summary

[L-01] The collateralAmount and getClaimAbleLBR have an accuracy error

Description

uint256 collateralAmount = (((eusdAmount * 1e18) / assetPrice) * (10000 - configurator.redemptionFee())) / 10000;

    function getClaimAbleLBR(address user) public view returns (uint256 amount) {
        if (time2fullRedemption[user] > lastWithdrawTime[user]) {
            amount = block.timestamp > time2fullRedemption[user] ? unstakeRatio[user] * (time2fullRedemption[user] - lastWithdrawTime[user]) : unstakeRatio[user] * (block.timestamp - lastWithdrawTime[user]);
        }
    }

The collateralAmount and getClaimAbleLBR have precision error in dividing before multiplying.

Recommendations

Multiply before divide

[L-02] The extraRatio and rewardRatio updates should pass through the time lock

Description

The calculation of incentives in EUSDMiningIncentives relies on extraRatio and rewardRatio, whose updates depend on the owner instead of the time lock.
If the owner updates the extraRatio and rewardRatio in an instant, such as reducing them, the user will not have time to claim the previous reward and will have less to claim later.
But a small number of people like searchers will listen to mempool to frontrun to claim the previous reward, which is unfair.
It should be updated with a time lock so that most users have time to claim their previous rewards.

Recommendations

The extraRatio and rewardRatio updates should pass through the time lock

[L-03] StakedLBRLpValue value can be manipulated

Description

StakedLBRLpValue using the balance in the uniswap lp pool to calculate the value is not a problem for internal temporary use, because even if staker temporarily manipulates the price to avoid liquidation, it would not be sustainable for long.
However, if an external contract invokes this interface, it may cause a loss. It is better to use reserve rather than balance to calculate the lp value.

Recommendations

Use reserve rather than balance to calculate the lp value.

[L-04] There are accuracy problems in EUSD internal accounting calculations

Description

The core problem is that the getSharesByMintedEUSD function has an accuracy error. In EUSD, due to the presence of burnShares, the exchangeRate of balance / share will increase. Assume exchangeRate increase to 1.1.
When a user or other contract interacts with EUSD, if the input amount is 1, it is converted to 0 shares, that is, the amount of the user account will not change, which can cause a series of problems for external contracts.
For example, tranfer and burn trigger the transfer event, which causes the external contract to think that the transfer has been made, but the actual user amount remains unchanged.

Recommendations

The precision error is reduced to 0 and a zero-value event should be triggered

[N-01] Decimals for collateral and USD may not be equal

Description

Code do not take into account collateral and usd decimals may not equal. Although the LST used now is all 18.

Recommendations

Add decimals convert.

[N-02] ProtocolRewardsPool getReward event parameter miscalculation

Description

    } else {
        if(peUSDBalance > 0) {
            peUSD.transfer(msg.sender, peUSDBalance);
        }
        ERC20 token = ERC20(configurator.stableToken());
        uint256 tokenAmount = (reward - eUSDShare - peUSDBalance) * token.decimals() / 1e18;
        token.transfer(msg.sender, tokenAmount);
        emit ClaimReward(msg.sender, EUSD.getMintedEUSDByShares(eUSDShare), address(token), reward - eUSDShare, block.timestamp);
    }
} else {
    emit ClaimReward(msg.sender, EUSD.getMintedEUSDByShares(eUSDShare), address(0), 0, block.timestamp);
}

For other stable token, tokenAmount is reward - eUSDShare - peUSDBalance, not reward - eUSDShare; For EUSD, tokenAmount is reward not 0 and address is address(EUSD).

Recommendations

Fix event parameter