search

sherlock-audit / 2023-02-gmx-judging

17 stars 11 forks source link

simon135 - If a user makes a tx from their wallet they can get frontrunned and lose their funds #206

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

simon135

high

If a user makes a tx from their wallet they can get frontrunned and lose their funds

Summary

If a user makes a tx from their wallet they can get frontrunned and lose their funds

Vulnerability Detail

When a user requests, they have to give funds first and then the user calls createDeposit function (just using a deposit as an ex:) which shows the difference in the balance of the bank and records the change. An attacker can front-run the users' request, make their own request, and take the funds. ex: bob sends 1 weth to the deposit vault (1 tx) bob makes a request (2tx) Alice after the 1 tx front runs the (2 tx) and steals the funds

Impact

an attacker can front-run and make requests with stolen funds

Code Snippet


        uint256 initialLongTokenAmount = depositVault.recordTransferIn(params.initialLongToken);
        uint256 initialShortTokenAmount = depositVault.recordTransferIn(params.initialShortToken);

Tool used

https://github.com/gmx-io/gmx-synthetics/blob/c4814a6c4c9269b9367fb6d462e30ff6f37480e5/contracts/deposit/DepositUtils.sol#L72-L73 Manual Review

Recommendation

have sending funds and creating orders and deposits into 1 tx but this issue is only for smart contract users (probably ui will solve this issue)