Lybra Finance - zaevlad's results

Lines of code

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

Vulnerability details

Impact

Maliciuos user can use executeFlashloan() and steal all available EUSD tokens.

Proof of Concept

Here is a vulnerable function:

    function executeFlashloan(FlashBorrower receiver, uint256 eusdAmount, bytes calldata data) public payable {
        uint256 shareAmount = EUSD.getSharesByMintedEUSD(eusdAmount);
        EUSD.transferShares(address(receiver), shareAmount);
        receiver.onFlashLoan(shareAmount, data);
        bool success = EUSD.transferFrom(address(receiver), address(this), 
        EUSD.getMintedEUSDByShares(shareAmount));
        require(success, "TF");

        uint256 burnShare = getFee(shareAmount);
        EUSD.burnShares(address(receiver), burnShare);
        emit Flashloaned(receiver, eusdAmount, burnShare);
    }

It allows to pass an arbitrary bytes calldata data as one of the arguments. Later it calls receiver, that can be PeUSDMainnetStableVision contract itself, with a malicious data.

...
receiver.onFlashLoan(shareAmount, data);
...

So malicious user can pass next arguments:

executeFlashloan( PeUSDMainnetStableVision , 1, abi.encodeWithSignature("approve(address,uint256)", attacker, amount));

Step by step:

1. Hacker pass malicious arguments;
2. PeUSDMainnetStableVision calculates the share amount and transfer it to itself;
3. Call an approve function in malicious data;
4. Transfer loaned token back to itself again, calculate fee and burn it;

Right after that hacker will be approved and be able to transfer all EUSD tokens from the contract.

Sorry for no tests included. I was little bit confused with setting up all params.

P.S. The same issue was in Damn Vulnerably DeFi Challenges (Truster).

Tools Used

Manual review.

Recommended Mitigation Steps

You can provide an additional checks to prohibit calls for approve functions or receiver should not be the same as PeUSDMainnetStableVision contract.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/configuration/LybraConfigurator.sol#L85 https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/configuration/LybraConfigurator.sol#L90 https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/governance/GovernanceTimelock.sol#L25 https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/governance/GovernanceTimelock.sol#L29

Vulnerability details

Impact

Core functions are unprotected due to a missing require check in the modifier, so malicious user can call them and break the protocol.

Proof of Concept

LybraConfigurator contract has two modifiers that should check the specific role of the user to have an ability to call some core functions. For example:

    modifier onlyRole(bytes32 role) {
        GovernanceTimelock.checkOnlyRole(role, msg.sender);
        _;
    }

However checkOnlyRole() only returns a bool whether a user has or has not a role.

    function checkOnlyRole(bytes32 role, address _sender) public view  returns(bool){
        return hasRole(role, _sender);
    }

AccessControl.sol

   function hasRole(bytes32 role, address account) public view virtual override returns (bool) {
        return _roles[role].members[account];
    }

So all functions that should be protected by these modifiers stay unprotected to anyone.

Tools Used

Manual review.

Recommended Mitigation Steps

Developers should add a require check to the modifiers, like:

    modifier onlyRole(bytes32 role) {
       require(GovernanceTimelock.checkOnlyRole(role, msg.sender), "Not a role");
        _;
    }

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L67 https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L181 https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/interfaces/IPeUSD.sol#L11 https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/PeUSD.sol#L8 https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L200

Vulnerability details

Impact

Users will not be able to mint or burn PeUSD tokens and use core functions of the protocol.

Proof of Concept

When a user want to deposit his assets via LybraPeUSDVaultBase contract he calls depositAssetToMint function that has internal call to _mintPeUSD().

MintPeUSD should forward a call to a mint function of PeUSD contract:

   ...
    PeUSD.mint(_onBehalfOf, _mintAmount);
   ...

However that specific mint() function can be found only in interface contract, but not in the main PeUSD contract. So any call will revert.

The same situation with a burn function when user wants to get his colateral back via repay(). Here is a call to the absence function:

   ...
   PeUSD.burn(_provider, amount - totalFee);
   ...

Tools Used

Manual review

Recommended Mitigation Steps

Provide a mint() and burn() functions with required logic for that token. Base on the developers needs it can be similar to EUSD token.

Assessed type

ERC20