cu5t0mPe0 - The way ArrakisPublicVaultRouter and ValantisHOTModulePublic calculate the deposit amount is inconsistent.

[sherlock-admin2]

cu5t0mPe0

high

The way ArrakisPublicVaultRouter and ValantisHOTModulePublic calculate the deposit amount is inconsistent.

Summary

ArrakisPublicVaultRouter rounds down when calculating the deposit amount, while ValantisHOTModulePublic rounds up. This discrepancy could result in the deposit amount for ValantisHOTModulePublic exceeding the allowance, causing the transaction to fail.

Vulnerability Detail

When a user makes a deposit, _getMintAmounts is first called to calculate the actual deposit amount. The calculation process rounds down.

https://github.com/sherlock-audit/2024-03-arrakis/blob/64a7dc6ccb5de2824870474a9f35fd3386669e89/arrakis-modular/src/ArrakisPublicVaultRouter.sol#L139-L141

Then call safeIncreaseAllowance to increase the module's allowance value.

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

In the module, the deposit amount is recalculated, and this time the calculation process rounds up. Therefore, the result calculated in the module is likely to be larger than the result calculated by ArrakisPublicVaultRouter, causing the allowance to be insufficient and the transaction to fail.

This vulnerability scenario primarily occurs in cases where the amounts cannot be evenly divided, making it highly likely to occur and potentially leading to a dos attack on the deposit function.

poc:

Place the following test into ArrakPublicVaultRouter.t.sol and run it, commenting out the transferFrom in ArrakisPublicVaultMock.sol::mint.

    function testHackAddLiquidity() public {
        // #region create public vault.

        ArrakisPublicVaultMock vault = new ArrakisPublicVaultMock();
        vault.setTokens(USDC, WETH);
        vault.mintLPToken(address(1), 1 ether);
        vault.setAmountToTake(0, 1e18);
        vault.setModule(address(vault));
        deal(WETH, address(vault), 1e18 + 232323);
        vault.setInits(0, 1e18 + 232323);

        // #endregion create public vault.
        // #region add vault to mock factory.

        factory.addPublicVault(address(vault));

        // #endregion add vault to mock factory.

        AddLiquidityData memory params = AddLiquidityData({
            amount0Max: 0,
            amount1Max: 1e17 + 131711,
            amount0Min: 0,
            amount1Min: 0,
            amountSharesMin: 0,
            vault: address(vault),
            receiver: address(0)
        });

        deal(WETH, address(this), 10e18);
        IERC20(WETH).approve(address(router), 10e18);
        uint256 amount0;
        uint256 amount1;
        uint256 sharesReceived;
        (amount0,amount1,sharesReceived) = router.addLiquidity(params);
        console.log("amount0: %d", amount0);
        console.log("amount1: %d", amount1);
    }

将下面测试文件放入ValantisHOTModulePublic.t.sol中运行，并且将setUp中的INIT1改为1e18 + 232323

    function testHackWithdrawReceiverAddressZero() public {
        // #region deposit.

        address depositor = vm.addr(10);
        uint256 proportion = (1e17 + 131711) * BASE / (1e18 + 232323);

        uint256 expectedAmount0 = 2000e6 / 2;
        uint256 expectedAmount1 = 100000000000131710;

        deal(USDC, depositor, expectedAmount0);
        deal(WETH, depositor, 10e18);

        vm.prank(depositor);
        IERC20(USDC).approve(address(module), expectedAmount0);
        vm.prank(depositor);
        IERC20(WETH).approve(address(module), 10e18);

        vm.prank(address(metaVault));
        uint256 amount0;
        uint256 amount1;
        (amount0, amount1) = module.deposit(depositor, proportion);
        console.log("amount0: %d", amount0);
        console.log("amount1: %d", amount1);
    }

results:

Ran 1 test for test/unit_tests/ArrakisPublicVaultRouter/ArrakisPublicVaultRouter.t.sol:ArrakisPublicVaultRouterTest
[PASS] testHackAddLiquidity() (gas: 1998173)
Logs:
  amount0: 0
  amount1: 100000000000131710

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.90s (2.49ms CPU time)

Ran 1 test for test/unit/ArrakisPublicVaultRouter.t.sol:ArrakisPublicVaultRouterTest

Ran 1 test for test/unit_tests/ValantisHOTModulePublic/ValantisHOTModulePublic.t.sol:ValantisHOTModuleTest
[PASS] testHackWithdrawReceiverAddressZero() (gas: 663214)
Logs:
  amount0: 200000001
  amount1: 100000000000131711

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.05s (2.70ms CPU time)

Ran 1 test suite in 2.09s (2.05s CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

It can be seen from the results that the amount deposited in the module is 100000000000131711, but the allowance value in the router will be 100000000000131710, causing the deposit to fail.

Impact

User cannot deposit

Code Snippet

https://github.com/sherlock-audit/2024-03-arrakis/blob/64a7dc6ccb5de2824870474a9f35fd3386669e89/arrakis-modular/src/ArrakisPublicVaultRouter.sol#L1228-L1229

Tool used

Manual Review

Recommendation

ArrakisPublicVaultRouter also needs to be rounded up

Duplicate of #54

[0xjuaan]

Escalate

This is a valid dupe of #54

[sherlock-admin3]

Escalate

This is a valid dupe of #54

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.

[cu5t0mPeo]

escalate dups of https://github.com/sherlock-audit/2024-03-arrakis-judging/issues/54

[sherlock-admin3]

escalate dups of https://github.com/sherlock-audit/2024-03-arrakis-judging/issues/54

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.

[Hash01011122]

Seems valid dup of #54 @0xffff11 @WangSecurity

[WangSecurity]

Agree with the escalation and planning to make it a duplicate of #54. But, only the first escalation by @0xjuaan will accepted and the second one by @cu5t0mPeo will be rejected, based on the following rule for escalation:

If there were previous escalations for the same issue (let's say it's the second escalation), it won't be accepted unless the new argument proposes new changes or provides stronger reasons for the changes requested earlier.

[WangSecurity]

Result: Medium Duplicate of #54

[sherlock-admin3]

Escalations have been resolved successfully!

Escalation status:

• 0xjuaan: accepted
• cu5t0mPeo: rejected