Lybra Finance - Neon2835's results

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/token/PeUSDMainnetStableVision.sol#L129-L139 https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/token/EUSD.sol#L228-L230

Vulnerability details

Impact

The executeFlashloan function of the PeUSDMainnet contract is used to provide users with the flash loan function. There is a loophole in the logic, and hackers can use this loophole to burn other people's eUSD token balance without permission.

Proof of Concept

Since the parameter FlashBorrower receiver of the executeFlashloan function can be designated as anyone, the flash loan system will charge a certain percentage of the loan fee (up to 10%) to receiver for each flash loan, the code is as follows:

EUSD.burnShares(address(receiver), burnShare);

When a hacker maliciously initiates a flash loan for a receiver contract, and the value of the eusdAmount parameter passed in is large enough, the receiver will be deducted a large amount of loan fees, and the hacker can burn a large amount of other people’s eUSD without permission the amount.

Let us analyze the design logic of the system itself step by step for discussion:

1. The flashloan fee of the PeUSDMainnet contract is collected by calling the burnShares function of the EUSD contract. Continue to read the code to find that the burnShares function of the EUSD contract has a very critical modifier onlyMintVault condition Judgment, so it is obvious that the PeUSDMainnet contract is the minter role of the EUSD contract (otherwise it will not be able to charge the flashloan fee).

2. Usually, when the transferFrom function is called, the ERC20 token needs to be approved by the spender before it can be used. But the transferFrom function in the EUSD contract is implemented like this:

function transferFrom(address from, address to, uint256 amount) public returns (bool) {
     address spender = _msgSender();
     if (!configurator. mintVault(spender)) {
         _spendAllowance(from, spender, amount);
     }
     _transfer(from, to, amount);
     return true;
}

The above code indicates that the miner of EUSD can call transferFrom arbitrarily without the user calling increaseAllowance for approval. The PeUSDMainnet contract is the minter of the EUSD contract, so line 133 of the PeUSDMainnet contract code: bool success = EUSD.transferFrom(address(receiver), address(this), EUSD.getMintedEUSDByShares(shareAmount)); can be executed without user approval.

3. In line 132 of the executeFlashloan function of the PeUSDMainnet contract: receiver.onFlashLoan(shareAmount, data);, if the receiver does not implement the onFlashLoan method, the EVM will revert, and the hacker will not be able to maliciously execute the attack. However, if the receiver contract simply declares the fallback() function, or its fallback() logic does not have a very robust judgment, then line 132 of the code can be easily bypassed. So is there really such a contract that just satisfies this condition? The answer is yes, for example this address: 0x32034276343de43844993979e5384d4b7e030934 ( etherscan: https://etherscan.io/address/0x32034276343de43844993979e5384d4b7e030934#code ), this address has 200,000 eUSD tokens, and declared the fallback function, its source code excerpts are as follows:

contract GnosisSafeProxy {
    // singleton always needs to be first declared variable, to ensure that it is at the same location in the contracts to which calls are delegated.
    // To reduce deployment costs this variable is internal and needs to be retrieved via `getStorageAt`
    address internal singleton;

    /// @dev Constructor function sets address of singleton contract.
    /// @param _singleton Singleton address.
    constructor(address _singleton) {
        require(_singleton != address(0), "Invalid singleton address provided");
        singleton = _singleton;
    }

    /// @dev Fallback function forwards all transactions and returns all received return data.
    fallback() external payable {
        // solhint-disable-next-line no-inline-assembly
        assembly {
            let _singleton := and(sload(0), 0xffffffffffffffffffffffffffffffffffffffff)
            // 0xa619486e == keccak("masterCopy()"). The value is right padded to 32-bytes with 0s
            if eq(calldataload(0), 0xa619486e00000000000000000000000000000000000000000000000000000000) {
                mstore(0, _singleton)
                return(0, 0x20)
            }
            calldatacopy(0, 0, calldatasize())
            let success := delegatecall(gas(), _singleton, 0, calldatasize(), 0, 0)
            returndatacopy(0, 0, returndatasize())
            if eq(success, 0) {
                revert(0, returndatasize())
            }
            return(0, returndatasize())
        }
    }
}

4. Assuming that the PeUSDMainnet contract flash loan fee rate is 5% at this time, the hacker maliciously calls the executeFlashloan function to initiate a flash loan with the address: 0x32034276343de43844993979e5384d4b7e030934, the function parameter uint256 eusdAmount = 4_000_000, and the calculated loan fee is 4_000_000 * 5% = 200_000, the 200_000 eUSD balance of the address 0x32034276343de43844993979e5384d4b7e030934 will be maliciously burned by hackers!

The following is the forge test situation I simulated locally

[PASS] testGnosisSafeProxy() (gas: 10044)
Traces:
  [10044] AttackTest::testGnosisSafeProxy() 
    ├─ [4844] GnosisSafeProxy::onFlashLoan() 
    │   ├─ [0] 0xd9Db270c1B5E3Bd161E8c8503c55cEABeE709552::onFlashLoan() [delegatecall]
    │   │   └─ ← ()
    │   └─ ← ()
    └─ ← ()

Test result: ok. 1 passed; 0 failed; finished in 972.63µs

The fallback function of the GnosisSafeProxy contract is allowed to be called without revert.

Tools Used

Visual Studio Code Foundry

Recommended Mitigation Steps

Optimize the flash loan logic of the executeFlashloan function of the PeUSDMainnet contract, remove the FlashBorrower receiverparameter, and set receiver to msg.sender, which means that a user can only initiate a flash loan for himself.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

Because the Configurator contract judges the user's authority wrongly, hackers can maliciously configure multiple core parameters of the system, which can cause many damages to the system, such as unlimited infinitely mint eUSD tokens, etc.

Proof of Concept

The Configurator contract is used to set various parameters and control functionalities of the Lybra Protocol. The vulnerable code block is as follows:

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

In line 86 of the Configurator contract, there is no judgment on the Boolean return value of the GovernanceTimelock.checkOnlyRole(role, msg.sender); statement, resulting in any user being able to bypass the permission check. If a hacker calls setMintVault to set himself as the minter of the eUSD contract, he can infinitely mint eUSD for himself.

Tools Used

Visual Studio Code Foundry

Recommended Mitigation Steps

Change line 86 of the Configurator contract: GovernanceTimelock.checkOnlyRole(role, msg.sender); to require(GovernanceTimelock.checkOnlyRole(role, msg.sender), "not authorized");

Assessed type

Access Control

Lines of code

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

Vulnerability details

Impact

The checkRole statement of the Configurator contract is invalid, and the permission verification can be bypassed. Hackers can tamper with tokenMiner parameters and infinitely mint esLBR tokens or LBR tokens.

Proof of Concept

The Configurator contract is used to set various parameters and control functionalities of the Lybra Protocol. The vulnerable code block is as follows(code link: https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/configuration/LybraConfigurator.sol#L90-L93 ):

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

In line 91 of the Configurator contract, there is no judgment on the Boolean return value of the GovernanceTimelock.checkRole(role, msg.sender); statement, which allows any user to bypass the permission check. If a hacker calls tokenMiner to set himself as the minter of the esLBR contract or the LBR contract, he can infinitely mint esLBR tokens or LBR tokens for himself.

Tools Used

Visual Studio Code Foundry

Recommended Mitigation Steps

Change line 91 of the Configurator contract:

GovernanceTimelock.checkRole(role, msg.sender);

to

require(GovernanceTimelock.checkRole(role, msg.sender), "not authorized");

Assessed type

Access Control