Lybra Finance - CrypticShepherd's results

Lines of code

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

Vulnerability details

Impact

IVault in configurator defines the interface to learn the vault type to be vaultType(), however it is getVaultType(). This will break any call that asks for the vault type, e.g. setSafeCollateralRation() and thus most of the protocol.

Recommended Mitigation Steps

Define and use correct interface.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

In LybraPeUSDVaultBase's _repay() function, fees are not correctly accounted for. This allows users to reach 0 debt even when repaying less than they owe. Repaying the full amount of getBorrowedOf() is not possible due to integer overflow.

Impact Details

Amount is deducted from borrowed and circulation on this line and the following one: https://github.com/code-423n4/2023-06-lybra/blob/5d70170f2c68dbd3f7b8c0c8fd6b0b2218784ea6/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L206, not accounting for fees in the correct way. E.g. if a user only repays fees accrued, his fees will be reduced by amount on this line https://github.com/code-423n4/2023-06-lybra/blob/5d70170f2c68dbd3f7b8c0c8fd6b0b2218784ea6/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L202 and then his borrowed will be reduced by the same amount: https://github.com/code-423n4/2023-06-lybra/blob/5d70170f2c68dbd3f7b8c0c8fd6b0b2218784ea6/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L206

Proof of Concept

POC run against this repo: https://github.com/CrypticShepherd/lybra_tests

    // POC: PeUSD accounting for repayments is incorrect
    function testRETHAccounting() public {
        vm.startPrank(owner);
        // deposit 2 rETH and mint 2000 PeUSD
        fakeRETH.approve(address(realRETHVault), 2*1e18);
        realRETHVault.depositAssetToMint(2*1e18, 2000*1e18);
        // let 100 days pass
        vm.warp(86400*100);
        uint256 debt = realRETHVault.getBorrowedOf(address(owner));
        // debt should now be more than 2k due to fees
        assertGt(debt, 2000*1e18);
        // repay 1600 PeUSD only
        realPeUSDMainnet.approve(address(realRETHVault), 1600*1e18);
        realRETHVault.burn(address(owner), 1600*1e18);
        // debt should have decreased by 1600 (this is where it currently fails)
        assertEq(realRETHVault.getBorrowedOf(address(owner)), debt - 1600*1e18);
        vm.stopPrank();
    }

Tools Used

Manual analysis, Foundry for POC.

Recommended Mitigation Steps

Deduct fees from repaid amount.

Assessed type

Math

Lines of code

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

Vulnerability details

Impact

The votingDelay and votingPeriod variables are set to 1 and 3 blocks respectively. This will not allow for governance processes to work properly.

Tools Used

Manual analysis.

Recommended Mitigation Steps

Increase voting delay and voting period.

Assessed type

Governance

Lines of code

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

Vulnerability details

Impact

rETH exchange rate is read from getExchangeRate(), not getExchangeRatio(). Current code will not work.

Source: https://etherscan.io/token/0xae78736Cd615f374D3085123A210448E74Fc6393#readContract

Recommended Mitigation Steps

Use correct interface.

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 exchange rate is read from exchangeRate(), not exchangeRatio(). Current code will not work.

Source: https://etherscan.io/address/0xa2e3356610840701bdf5611a53974510ae27e2e1#readProxyContract

Recommended Mitigation Steps

Use correct interface.

Assessed type

Other

LBR on Arbitrum: unnecessary dependencies and code

The LBR token implementation refers to the configurator, which will, based on my current my understanding, only be deployed on Ethereum mainnet for V2.

https://github.com/code-423n4/2023-06-lybra/blob/26915a826c90eeb829863ec3851c3c785800594b/contracts/lybra/token/LBR.sol#L19

With no configurator, mint() and burn() in the LBR contract on Arbitrum will always revert, as the tokenMiner check at the beginning of those functions cannot be done.

Filing this as a QA finding as it's easy to work around and doesn't seem to cause any bigger issues, but still appears quite inelegant.

Cleaner way to implement OFTV2

Instead of Copy-Pasting some OFTV2 implementation code into LBR.sol, PeUSD.sol and PeUSDMainnetStableVision.sol, it would be cleaner to base those contracts off OFTV2 given here: https://github.com/LayerZero-Labs/solidity-examples/blob/main/contracts/token/oft/v2/OFTV2.sol

PeUSD mainnet: zero address check in mint() is redundant

https://github.com/code-423n4/2023-06-lybra/blob/5d70170f2c68dbd3f7b8c0c8fd6b0b2218784ea6/contracts/lybra/token/PeUSDMainnetStableVision.sol#L64

This check is performed in _mint() of the OZ ERC20 implementation (observe that burn() in the same contract doesn't check for the 0 address).

Flashloan callback interface spec in PeUSDMainnetStableVision

Interface spec declares "amount of token":

https://github.com/code-423n4/2023-06-lybra/blob/5d70170f2c68dbd3f7b8c0c8fd6b0b2218784ea6/contracts/lybra/token/PeUSDMainnetStableVision.sol#L23

But actual value given is "amount of shares":

https://github.com/code-423n4/2023-06-lybra/blob/5d70170f2c68dbd3f7b8c0c8fd6b0b2218784ea6/contracts/lybra/token/PeUSDMainnetStableVision.sol#L132

EUSD.mint() is supposed to return newTotalShares, but doesn't

Function doesn't contain a return statement: https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/token/EUSD.sol#L411