0xlookman - 0xlookman - `ArrakisPublicVaultRouter.sol::swapAndAddLiquidity` can be used to steal `eth`\ native token funds from this contract

[sherlock-admin3]

0xlookman

high

0xlookman - ArrakisPublicVaultRouter.sol::swapAndAddLiquidity can be used to steal eth\ native token funds from this contract

---

0xlookman

High

ArrakisPublicVaultRouter.sol::swapAndAddLiquidity can be used to steal eth\ native token funds from this contract

---

Summary

Calling ArrakisPublicVaultRouter.sol::swapAndAddLiquidity with msg.value and params_.addData.amountMax zero with the native token vault, allows a user to bypass checks hence stealing money from the contract.

Vulnerability Detail

In ArrakisPublicVaultRouter.sol the function swapAndAddLiquidity allows users to swap and add Liquidity to the pool.

But an attacker can exploit this function when he pretends to be using the vault with the native token to steal funds kept in this contract.

The function checks whether one of the swap tokens is the native token and checks if the params_.addData.amountMax is zero and msg.value amount of this call is the same as the one specified in the parameters in the code below.

// #endregion checks.

        if (
            token0 == nativeToken && params_.addData.amount0Max > 0
                && msg.value != params_.addData.amount0Max
        ) {
            revert NotEnoughNativeTokenSent();
        }

        if (
            token1 == nativeToken && params_.addData.amount1Max > 0
                && msg.value != params_.addData.amount1Max
        ) {
            revert NotEnoughNativeTokenSent();
        }

        // #region interactions. 

An attacker can bybass this when he uses the native token vault but sends an empty or zero msg.value and params_.addData.amountMax to the contract. This will bypass the above check.

The attacker can set the amountInSwap to be an amount already in the contract.

He can send a few tokens of the other token that are equal to that token's params_.addData.amountMax to bypass the check the checks below.

if (
            params_.addData.amount0Max == 0
                && params_.addData.amount1Max == 0
        ) {
            revert EmptyMaxAmounts();
        }

and this one

if (params_.addData.amount0Max > 0 && token0 != nativeToken) {
            IERC20(token0).safeTransferFrom(
                msg.sender, address(this), params_.addData.amount0Max
            );
        }
        if (params_.addData.amount1Max > 0 && token1 != nativeToken) {
            IERC20(token1).safeTransferFrom(
                msg.sender, address(this), params_.addData.amount1Max
            );
        }

He can use a contrary params_.swapData.zeroForOne param.

He can ensure the RouterSwapExecutor contract returns zero for the native token's amountDiff hence shares will be minted for the sender.

These functions are nonRentrant but the contract has a receive() payable function which can receive the native token from other sources and it is these funds an attacker can exploit.

Impact

Someone can steal funds from the protocol and get a share of the vault tokens that he has not paid for while giving away a few tokens.

Code Snippet

https://github.com/sherlock-audit/2024-03-arrakis/blob/main/arrakis-modular/src/ArrakisPublicVaultRouter.sol#L200

Tool used

Manual Review

Recommendation

Add extra checks to ensure when a person is swapping for the native to the other token, the params_.addData.amountMax of the native should not be zero.