Flash Loan Vulnerability: Potential Unauthorized Transfers during Flash Loan Execution

[hats-bug-reporter]

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.

Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/src/router/Dispatcher.sol

Lines 256 to 263 in 06b05fc

	flashloanLender = lender;
	IERC3156FlashLender(lender).flashLoan(
	IERC3156FlashBorrower(address(this)),
	token,
	amount,
	data
	);
	flashloanLender = address(0);

Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/src/router/Router.sol

Line 277 in 06b05fc

this.execute(commands, inputs); // https://ethereum.stackexchange.com/questions/103437/converting-bytes-memory-to-bytes-calldata

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

Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/src/tokens/PrincipalToken.sol

Lines 465 to 466 in 06b05fc

	function flashLoan(
	IERC3156FlashBorrower _receiver,

Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/src/tokens/PrincipalToken.sol

Lines 481 to 483 in 06b05fc

	if (_receiver.onFlashLoan(msg.sender, _token, _amount, fee, _data) != ON_FLASH_LOAN)
	revert FlashLoanCallbackFailed();

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

Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/src/router/Router.sol

Lines 263 to 275 in 06b05fc

	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();
	}

Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/src/router/Router.sol

Lines 284 to 288 in 06b05fc

	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);
	}

Scenario

1. The victim initiates a flash loan and provides various commands to be executed before loan repayment.
2. The flashloanLender variable is set to the trusted lender at the start of the flash loan.
3. 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.
4. The onFlashLoan function will not check the initiator and will execute the command as the flashloanLender check passes.
5. 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:

1. 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.
2. 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

[yanisepfl]

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

[0xmahdirostami]

@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:

• could be any untrusted wrapper

Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/src/router/Dispatcher.sol

Lines 151 to 153 in 06b05fc

	IERC20(vault).forceApprove(wrapper, vaultShares);
	ISpectra4626Wrapper(wrapper).wrap(vaultShares, recipient, minWrapperShares);
	IERC20(vault).forceApprove(wrapper, 0);

• could be any untrusted ibt

Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/src/router/Dispatcher.sol

Lines 177 to 179 in 06b05fc

	IERC20(asset).forceApprove(ibt, assets);
	IERC4626(ibt).deposit(assets, recipient);
	IERC20(asset).forceApprove(ibt, 0);

• could be any unregistered PT

Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/src/router/Dispatcher.sol

Lines 192 to 198 in 06b05fc

	bool isRegisteredPT = IRegistry(registry).isRegisteredPT(pt);
	if (isRegisteredPT) {
	_ensureApproved(asset, pt, assets);
	} else {
	IERC20(asset).forceApprove(pt, assets);
	}
	IPrincipalToken(pt).deposit(assets, ptRecipient, ytRecipient, minShares);

• could be any actor in the Kyber swap call

• could be unknown Curve pool

Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/src/router/Dispatcher.sol

Lines 131 to 140 in 06b05fc

	IERC20(token).forceApprove(pool, amountIn);
	ICurvePool(pool).exchange(
	i,
	j,
	amountIn,
	minAmountOut,
	false, // Do not use ETH
	recipient
	);
	IERC20(token).forceApprove(pool, 0);

[yanisepfl]

Hello again,

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

This is impossible since a flash loan execution happens atomically.

No one can execute your scenario's point 4 before before the transaction executed at 3 finished.

Thanks again and have a good day