whitehair0330 - `ArrakisPublicVaultRouter.addLiquidity()` function can frequently revert due to rounding errors.

[sherlock-admin2]

whitehair0330

high

ArrakisPublicVaultRouter.addLiquidity() function can frequently revert due to rounding errors.

Summary

In the adding liquidity functions, the deposited amounts of token0 and token1 are calculated twice. Because of the rounding error, these amounts have different values each other. So, ArrakisPublicVaultRouter.addLiquidity() function will revert very often.

Vulnerability Detail

In L139 of the ArrakisPublicVaultRouter.addLiquidity() function, the received amounts of shares and the deposited amounts of tokens are calculated by using the _getMintAmounts() function.

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

        function addLiquidity(AddLiquidityData memory params_)
            external
            payable
            nonReentrant
            whenNotPaused
            onlyPublicVault(params_.vault)
            returns (
                uint256 amount0,
                uint256 amount1,
                uint256 sharesReceived
            )
        {
                [...]
139         (sharesReceived, amount0, amount1) = _getMintAmounts(
                params_.vault, params_.amount0Max, params_.amount1Max
            );
                [...]
            if (token0 != nativeToken && amount0 > 0) {
                IERC20(token0).safeTransferFrom(
162                 msg.sender, address(this), amount0
                );
            }

            if (token1 != nativeToken && amount1 > 0) {
                IERC20(token1).safeTransferFrom(
168                 msg.sender, address(this), amount1
                );
            }

            _addLiquidity(
                params_.vault,
174             amount0,
175             amount1,
176             sharesReceived,
                params_.receiver,
                token0,
                token1
            );
                [...]
        }

After transferring tokens from msg.sender to itself, IArrakisMetaVaultPublic(vault_).mint() is called.

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

    function _addLiquidity(
        [...]
884         IERC20(token0_).safeIncreaseAllowance(module, amount0_);
        [...]
891         IERC20(token1_).safeIncreaseAllowance(module, amount1_);
        [...]
898     IArrakisMetaVaultPublic(vault_).mint{value: valueToSend}(
            shares_, receiver_
        );

In the ArrakisMetaVaultPublic.mint() function, the proportion of the minted shares is calculated.

https://github.com/sherlock-audit/2024-03-arrakis/blob/main/arrakis-modular/src/ArrakisMetaVaultPublic.sol#L51-L74

        function mint(
                [...]
58          uint256 proportion = FullMath.mulDivRoundingUp(
                shares_, BASE, supply > 0 ? supply : 1 ether
            );
                [...]
71          (amount0, amount1) = _deposit(proportion);
                [...]
        }

https://github.com/sherlock-audit/2024-03-arrakis/blob/main/arrakis-modular/src/ArrakisMetaVaultPublic.sol#L137-L154

        function _deposit(uint256 proportion_)
                [...]
            bytes memory data = abi.encodeWithSelector(
                IArrakisLPModulePublic.deposit.selector,
                msg.sender,
147             proportion_
            );

            bytes memory result = payable(address(module))
151             .functionCallWithValue(data, msg.value);
                [...]
        }

And, the amounts of tokens are calculated, which are transferred from ArrakisPublicVaultRouter to alm.

https://github.com/sherlock-audit/2024-03-arrakis/blob/main/arrakis-modular/src/modules/ValantisHOTModulePublic.sol#L35-L96

        function deposit(
            address depositor_,
            uint256 proportion_
        )
            external
            payable
            onlyMetaVault
            whenNotPaused
            nonReentrant
            returns (uint256 amount0, uint256 amount1)
        {
                    [...]
                (uint256 _amt0, uint256 _amt1) = pool.getReserves();
                    [...]
                amount0 =
72                  FullMath.mulDivRoundingUp(proportion_, _amt0, BASE);
                amount1 =
74                  FullMath.mulDivRoundingUp(proportion_, _amt1, BASE);
                    [...]
79          token0.safeTransferFrom(depositor_, address(this), amount0);
80          token1.safeTransferFrom(depositor_, address(this), amount1);
                    [...]
        }

However, these amounts can be larger than the amounts calculated in the _getMintAmounts() function.

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

        function _getMintAmounts(
            address vault_,
            uint256 maxAmount0_,
            uint256 maxAmount1_
        )
            internal
            view
            returns (
                uint256 shareToMint,
                uint256 amount0ToDeposit,
                uint256 amount1ToDeposit
            )
        {
            // TODO check rounding !!!!
            (uint256 amount0, uint256 amount1) =
                IArrakisMetaVault(vault_).totalUnderlying();

            uint256 supply = IERC20(vault_).totalSupply();

            if (amount0 == 0 && amount1 == 0) {
                (amount0, amount1) = IArrakisMetaVault(vault_).getInits();
                supply = 1 ether;
            }

            uint256 proportion0 = amount0 == 0
                ? type(uint256).max
1220            : FullMath.mulDiv(maxAmount0_, BASE, amount0);
            uint256 proportion1 = amount1 == 0
                ? type(uint256).max
1223            : FullMath.mulDiv(maxAmount1_, BASE, amount1);

            uint256 proportion =
                proportion0 < proportion1 ? proportion0 : proportion1;

1228        amount0ToDeposit = FullMath.mulDiv(amount0, proportion, BASE);
1229        amount1ToDeposit = FullMath.mulDiv(amount1, proportion, BASE);
1230        shareToMint = FullMath.mulDiv(proportion, supply, BASE);
        }

The amount calculated in the ArrakisPublicVaultRouter._getMintAmounts() function is: allowedAmount = (amount0 * BASE / reserve0 ) * reserve0 / BASE;

The amount calculated in the ValantisHOTModulePublic.deposit() function is: shares = (amount0 * BASE / supply ) * reserve0 / BASE; transferredAmount = roundingUp((roundingUp(shares * BASE / supply) * reserve0 / BASE));

transferredAmount is smaller than allowedAmount very often due to rounding errors, thus ArrakisPublicVaultRouter.addLiquidity() will be reverted.

For example: Assume that amount0 = 1000, reserve0 = 1e18 + 1, supply = 1e18. Then allowedAmount = 999, transferredAmount = 1000. So, transferredAmount > allowedAmount, which results in reverting.

Impact

All functions adding liquidity such as addLiquidity() and swapAndAddLiquidity() can frequently revert due to rounding errors.

Code Snippet

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

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

https://github.com/sherlock-audit/2024-03-arrakis/blob/main/arrakis-modular/src/ArrakisMetaVaultPublic.sol#L51-L74

https://github.com/sherlock-audit/2024-03-arrakis/blob/main/arrakis-modular/src/modules/ValantisHOTModulePublic.sol#L35-L96

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

Tool used

Manual Review

Recommendation

The ArrakisPublicVaultRouter._getMintAmounts() function should be modified to return correct needed amounts for minting.

Duplicate of #54

[whitehair0330]

Escalate

This issue should be considered high severity. ArrakisPublicVaultRouter.addLiquidity() will be reverted almost 100%. So, depositing through ArrakisPublicVaultRouter is almost impossible, which is a break of core functionality.

[sherlock-admin3]

Escalate

This issue should be considered high severity. ArrakisPublicVaultRouter.addLiquidity() will be reverted almost 100%. So, depositing through ArrakisPublicVaultRouter is almost impossible, which is a break of core functionality.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[WangSecurity]

Main report of this family #54 is also escalated to increase severity, and as of now, I'm planning to accept it, hence all the duplicates of it (including this report) will be upgraded to high severity as well, so this escalation will be rejected.

Advice for future, if you want to increase severity or invalidate the entire family, it's better to escalate the main report. If you believe one of the duplicates is not a separate issue, then escalate that specific report.

[WangSecurity]

Result: Medium Duplicate of #54

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• whitehair0330: rejected