Flash Loan Vulnerability: Potential Unauthorized Transfers during Flash Loan Execution

Github username: @0xmahdirostami Twitter username: 0xmahdirostami Submission hash (on-chain): 0x4a7a612fd62ef9364c23b61b1f39f0277f2ed86f38f288f403cc2da0f79386c3 Severity: high

Description:

description

In the current flash loan implementation, the flashloanLender variable is set to the lender's address when a flash loan is initiated and remains unchanged until the loan is completed. This creates a vulnerability where, during subsequent commands in the flash loan transaction, malicious actors could potentially transfer funds without the user’s knowledge.

The vulnerability exists because the onFlashLoan function does not verify that the flash loan initiator matches the Router contract’s address. Since flashloanLender remains unchanged throughout the loan, a malicious actor could execute steps that involve calling the lender’s flashLoan function with the Router contract as the receiver, causing an unauthorized transfer of funds from the user to the lender.

details

after a flash loan command, flashloanLender will be assigned to the lender address, and during all upcoming commands, this value will be the same until the end of flash loan execution.

https://github.com/hats-finance/Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/blob/06b05fc261fe59b1fe25e308a10a16459289a4be/src/router/Dispatcher.sol#L256-L263

https://github.com/hats-finance/Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/blob/06b05fc261fe59b1fe25e308a10a16459289a4be/src/router/Router.sol#L277

in any upcoming steps, anyone could call a lender with a router address as a receiver.

https://github.com/hats-finance/Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/blob/06b05fc261fe59b1fe25e308a10a16459289a4be/src/tokens/PrincipalToken.sol#L465-L466

https://github.com/hats-finance/Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/blob/06b05fc261fe59b1fe25e308a10a16459289a4be/src/tokens/PrincipalToken.sol#L481-L483

as flashloanLender has not changed yet and the onFlashLoan function doesn't check initiator, anyone could transfer funds from msgSender to lender

https://github.com/hats-finance/Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/blob/06b05fc261fe59b1fe25e308a10a16459289a4be/src/router/Router.sol#L263-L275

https://github.com/hats-finance/Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/blob/06b05fc261fe59b1fe25e308a10a16459289a4be/src/router/Router.sol#L284-L288

Scenario

The victim initiates a flash loan and provides various commands to be executed before loan repayment.
The flashloanLender variable is set to the trusted lender at the start of the flash loan.
During the execution of these commands, the flashloanLender remains unchanged, allowing a malicious target to exploit this state by calling lender::flashLoan with the Router contract as _receiver.
The onFlashLoan function will not check the initiator and will execute the command as the flashloanLender check passes.
As a result, funds can be transferred from the victim's balance to the lender, with the user unknowingly incurring additional fees if they approved the router.

Impact

This vulnerability allows unauthorized fund transfers.

mitigation

To secure the flash loan mechanism, implement the following changes:

Reset flashloanLender after the first call from the lender: Instead of only clearing flashloanLender upon flash loan completion, reset it immediately after the first interaction from the lender.
Check the initiator in onFlashLoan: Ensure that onFlashLoan only executes commands if the initiator is the Router contract itself. This means the contract should only accept calls initiated by address(this).

Mitigation Code:

/**
     * @inheritdoc IERC3156FlashBorrower
     */
    function onFlashLoan(
-        address /* initiator */,
+        address initiator ,
        address _token,
        uint256 _amount,
        uint256 _fee,
        bytes calldata _data
    ) external returns (bytes32) {
        if (msgSender == address(0)) {
            revert DirectOnFlashloanCall();
        }
        if (msg.sender != flashloanLender) {
            revert UnauthorizedOnFlashloanCaller();
        }
+   if (initiator != address(this)) {
+            revert UnauthorizedOnFlashloanCaller();
        }
+   flashloanLender = address(0); // Clear flashloanLender immediately

        (bytes memory commands, bytes[] memory inputs) = abi.decode(_data, (bytes, bytes[]));
        this.execute(commands, inputs); // https://ethereum.stackexchange.com/questions/103437/converting-bytes-memory-to-bytes-calldata

Hello,

We classified this issue as invalid because flashloanLender can only be PTs and therefore the method onFlashLoan can only be called through the PT contract.

Thanks

@yanisepfl

Hello,

flashloanLender can only be PTs and therefore the method onFlashLoan can only be called through the PT contract.

yes, this is true, but the issue is not about malicious flashloanLender.

The issue is that anyone could call 'flashloanLender:flashLoan' before flash loan repayments.

During flash loan repayment, any actor could call flashloanLender::flashLoan and set a router as a receiver. FlashloanLender will call onFlashLoan on the router and msgSender will lose some approved tokens for a fee.

Consider the following scenario:

Bob wants to get a flash loan and execute some commands before flash loan repayment.

During a repayment, any other actor could call flashLoan on trusted flashloanLender and set up a router as a receiver. After that, as there is no verification on the router::onFlashLoan function regarding the initiator, the user will lose their approved tokens.

    function flashLoan(
@@@@>         IERC3156FlashBorrower _receiver,
        address _token,
        uint256 _amount,
        bytes calldata _data
    ) external override whenNotPaused returns (bool) {
        if (_amount > maxFlashLoan(_token)) revert FlashLoanExceedsMaxAmount();

        uint256 fee = flashFee(_token, _amount);
        _updateFees(fee);

        // Initiate the flash loan by lending the requested IBT amount
        address _ibt = ibt;
        IERC20(_ibt).safeTransfer(address(_receiver), _amount);

        // Execute the flash loan
@@@@>       if (_receiver.onFlashLoan(msg.sender, _token, _amount, fee, _data) != ON_FLASH_LOAN)
            revert FlashLoanCallbackFailed();

        // Repay the debt + fee
        IERC20(_ibt).safeTransferFrom(address(_receiver), address(this), _amount + fee);

        return true;
    }

    function onFlashLoan(
@@@@>        address /* initiator */,
        address _token,
        uint256 _amount,
        uint256 _fee,
        bytes calldata _data
    ) external returns (bytes32) {
        if (msgSender == address(0)) {
            revert DirectOnFlashloanCall();
        }
         if (msg.sender != flashloanLender) {
            revert UnauthorizedOnFlashloanCaller();
        }
        (bytes memory commands, bytes[] memory inputs) = abi.decode(_data, (bytes, bytes[]));
        this.execute(commands, inputs); // https://ethereum.stackexchange.com/questions/103437/converting-bytes-memory-to-bytes-calldata
        uint256 repayAmount = _amount + _fee;
        uint256 allowance = IERC20(_token).allowance(address(this), msg.sender);
        if (allowance < repayAmount) {
            // Approve the lender to pull the funds if needed
            IERC20(_token).forceApprove(msg.sender, repayAmount);
        }
        uint256 balance = IERC20(_token).balanceOf(address(this));
        if (balance < repayAmount) {
            // Collect remaining debt from the original sender if needed
@@@@>            IERC20(_token).safeTransferFrom(msgSender, address(this), repayAmount - balance);
        }
        return ON_FLASH_LOAN;

There is no check to verify who the initiator is. The initiator must be address(this), meaning the contract should only accept flash loan calls that are initiated by the router itself.

The final question here will be who will be the other actor: