Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SwapAction.sol#balancerSwap does not support native ETH as input token. #70

Open
c4-bot-1 opened this issue Aug 15, 2024 · 2 comments
Open
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working M-37 primary issue Highest quality submission among a set of duplicates 🤖_primary AI based primary recommendation 🤖_121_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sufficient quality report This report is of sufficient quality

Comments

@c4-bot-1
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2024-07-loopfi/blob/main/src/proxy/SwapAction.sol#L263

Vulnerability details

Impact

SwapAction.sol#balancerSwap does not support native ETH as input token.

Bug Description

SwapAction is used to swap tokens using Balancer/Uniswap or join/exit a Pendle pool. Pendle accepts native ETH as input token when joining a Pendle pool, and Balancer accepts native ETH during swap.

We can check that the SwapAction contract also supports passing native ETH as input token, because 1) swap() function, which serves as the entry function, is payable; 2) pendleJoin() passes msg.value along when calling pendleRouter.addLiquiditySingleToken().

However, the issue is that when performing a balancer swap by balancerVault.batchSwap, the msg.value is not passed along.

    function swap(SwapParams memory swapParams) public payable returns (uint256 retAmount) {
        if (swapParams.swapProtocol == SwapProtocol.BALANCER) {
            (bytes32[] memory poolIds, address[] memory assetPath) = abi.decode(
                swapParams.args,
                (bytes32[], address[])
            );
            retAmount = balancerSwap(
                swapParams.swapType,
                swapParams.assetIn,
                poolIds,
                assetPath,
                swapParams.amount,
                swapParams.limit,
                swapParams.recipient,
                swapParams.deadline
            );
        } else if (swapParams.swapProtocol == SwapProtocol.UNIV3) {
            retAmount = uniV3Swap(
                swapParams.swapType,
                swapParams.assetIn,
                swapParams.amount,
                swapParams.limit,
                swapParams.recipient,
                swapParams.deadline,
                swapParams.args
            );
        } else if (swapParams.swapProtocol == SwapProtocol.PENDLE_IN) {
            retAmount = pendleJoin(swapParams.recipient, swapParams.limit, swapParams.args);
        } else if (swapParams.swapProtocol == SwapProtocol.PENDLE_OUT) {
            retAmount = pendleExit(swapParams.recipient, swapParams.amount, swapParams.args);
        } else revert SwapAction__swap_notSupported();
        // Transfer any remaining tokens to the recipient
        if (swapParams.swapType == SwapType.EXACT_OUT && swapParams.recipient != address(this)) {
            IERC20(swapParams.assetIn).safeTransfer(swapParams.recipient, swapParams.limit - retAmount);
        }
    }

    function balancerSwap(
        SwapType swapType,
        address assetIn,
        bytes32[] memory poolIds,
        address[] memory assets,
        uint256 amount,
        uint256 limit,
        address recipient,
        uint256 deadline
    ) internal returns (uint256) {
        ...
        return
            abs(
                // @auditnote: BUG. Does not pass msg.value.
@>              balancerVault.batchSwap(
                    kind,
                    swaps,
                    assets,
                    FundManagement({
                        sender: address(this),
                        fromInternalBalance: false,
                        recipient: payable(recipient),
                        toInternalBalance: false
                    }),
                    limits,
                    deadline
                )[pathLength]
            );
    }

    function pendleJoin(address recipient, uint256 minOut, bytes memory data) internal returns (uint256 netLpOut){
        (
            address market,
            ApproxParams memory guessPtReceivedFromSy,
            TokenInput memory input,
            LimitOrderData memory limit
        ) = abi.decode(data, (address, ApproxParams, TokenInput , LimitOrderData));
        
        if (input.tokenIn != address(0)) {
                input.netTokenIn = IERC20(input.tokenIn).balanceOf(address(this));
                IERC20(input.tokenIn).forceApprove(address(pendleRouter),input.netTokenIn);
            }

        (netLpOut,,) = pendleRouter.addLiquiditySingleToken{value: msg.value}(recipient, market, minOut, guessPtReceivedFromSy, input, limit);
    }

Proof of Concept

N/A

Tools Used

Manual Review

Recommended Mitigation Steps

-              balancerVault.batchSwap(
+              balancerVault.batchSwap{value: msg:value}(

Assessed type

Other

@c4-bot-1 c4-bot-1 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Aug 15, 2024
c4-bot-9 added a commit that referenced this issue Aug 15, 2024
@c4-bot-12 c4-bot-12 added 🤖_121_group 🤖_primary AI based primary recommendation 🤖_121_group AI based duplicate group recommendation and removed 🤖_121_group labels Aug 15, 2024
@howlbot-integration howlbot-integration bot added primary issue Highest quality submission among a set of duplicates sufficient quality report This report is of sufficient quality labels Aug 20, 2024
@0xtj24 0xtj24 added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Sep 19, 2024
@c4-judge
Copy link
Contributor

c4-judge commented Oct 1, 2024

koolexcrypto marked the issue as satisfactory

@c4-judge c4-judge added satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report labels Oct 1, 2024
@c4-judge
Copy link
Contributor

c4-judge commented Oct 2, 2024

koolexcrypto marked the issue as selected for report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working M-37 primary issue Highest quality submission among a set of duplicates 🤖_primary AI based primary recommendation 🤖_121_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

5 participants