swapETHForYt would always revert due to a wrong check in receiveFlashLoan function.

Summary

There is a check in receiveFlashLoan function that is always true and causes the receiveFlashLoan function to revert thus making the swapETHForYt useless.

Vulnerability Detail

Following is reveiveFlashLoan function

 function receiveFlashLoan(
        IERC20[] calldata, /* tokens */
        uint256[] calldata amounts,
        uint256[] calldata feeAmounts,
        bytes calldata /* userData */
    ) external {
        // CHECK
        // Note: Call only through `swapETHForYt` && from the Vault should be allowed.
        // This ensures that the function call is invoked by `swapETHForYt` entry point.
        // Checking `msg.sender == address(vault)` may not be sufficient as the call may be initiated by other contracts and pass arbitrary data.
        assembly {
            let ctx := tload(TSLOT_0)
            tstore(TSLOT_0, 0) // Delete the authorization (flag=address(0))
            if iszero(eq(ctx, address())) {
                mstore(0x00, 0x5c501941) // `MetapoolRouterUnauthorized()`
                revert(0x1c, 0x04)
            }
        }

        address pt = TransientStorage.tloadAddress(TSLOT_CB_DATA_PT);
        address metapool = TransientStorage.tloadAddress(TSLOT_CB_DATA_METAPOOL);

        // Issue PT tokens using the WETH
        if (_isApproved(address(WETH9), pt) == 0) {
            _setApproval(address(WETH9), pt);
            WETH9.approve(pt, type(uint256).max);
        }
        uint256 wethDeposit = amounts[0];
        uint256 pyIssued = ITranche(pt).issue(address(this), wethDeposit);

        // Swap the PT for the base pool token on the Curve metapool
        ITranche(pt).transfer(metapool, pyIssued);
        uint256 basePoolTokenOut =
            Twocrypto(metapool).exchange_received(PEGGED_PT_INDEX, BASE_POOL_INDEX, pyIssued, 0, address(this));

        // Swap the received base pool token for ETH on the NapierPool
        uint256 wethReceived = triLSTPool.swapExactBaseLpTokenForUnderlying(basePoolTokenOut, address(this));

        // Unreasonable situation: Received more WETH than sold
        if (wethReceived > wethDeposit) revert Errors.MetapoolRouterNonSituationSwapETHForYt();

        // Calculate the amount of ETH spent in the swap
        uint256 repayAmount = wethDeposit + feeAmounts[0];
        uint256 spent = repayAmount - wethReceived; // wethDeposit + feeAmounts[0] - wethReceived

        // Revert if the ETH spent exceeds the specified maximum
        if (spent > TransientStorage.tloadU256(TSLOT_CB_DATA_MAX_ETH_SPENT)) {
            revert Errors.MetapoolRouterExceededLimitETHIn();
        }

        uint256 remaining = TransientStorage.tloadU256(TSLOT_CB_DATA_VALUE) - spent;
        if (repayAmount > remaining) revert Errors.MetapoolRouterInsufficientETHRepay(); // Can't repay the flash loan

        // Temporarily store a return value of `swapETHForYt` function across the call context
        assembly {
            tstore(TSLOT_1, spent)
        }

        // Transfer the YT tokens to the recipient
        IERC20(ITranche(pt).yieldToken()).transfer(TransientStorage.tloadAddress(TSLOT_CB_DATA_RECEIPIENT), pyIssued);

        // Repay the flash loan
        WETH9.transfer(msg.sender, repayAmount);

        // Unwrap and send the remaining WETH back to the sender
        _unwrapWETH(TransientStorage.tloadAddress(TSLOT_CB_DATA_SENDER), remaining);
    }

Issue lies in the following if condition

   if (repayAmount > remaining) revert Errors.MetapoolRouterInsufficientETHRepay(); // Can't repay the flash loan

Now the above condition after substituting and rearranging is essentially equal to

if ( 2 repayAmount > msg.value(sent by the user) + wethReceived) then revert

repayAmount = wethDeposit + feeAmounts[0] also above there is a if condition

 if (wethReceived > wethDeposit) revert Errors.MetapoolRouterNonSituationSwapETHForYt();

From this we can see that the wethDeposit is always greater than the wethReceived so RepayAmount is always greater than the wethReceived. Also RepayAmount would obviously be greater than the msg,value otherwise it doesn't makes any sense to call the flash loan function on the vault. So repayAmount > wethReceived repayAmount > msg.value Adding above two we can clearly see that 2 repayAmount > msg,value(sent by user) + wethReceived Therefore the following if condition would always revert.

 if (repayAmount > remaining) revert Errors.MetapoolRouterInsufficientETHRepay(); // Can't repay the flash loan

Impact

Breaks the swapEthForYt functionality

Code Snippet

https://github.com/sherlock-audit/2024-05-napier-update/blob/main/metapool-router/src/MetapoolRouter.sol#L333

Tool used

Manual Review

Recommendation

Remove the if condition. Other code logic is sufficient to check whether the flash loan can be paid or not because remaining amount is calculated as follows uint256 remaining = TransientStorage.tloadU256(TSLOT_CB_DATA_VALUE) - spent; Now if spent the flash loan can't be paid then remaining value would become negative and as it is uint256 it would revert.

Duplicate of #36

sherlock-audit / 2024-05-napier-update-judging

Varun_05 - swapETHForYt would always revert due to a wrong check in receiveFlashLoan function. #71