search

sherlock-audit / 2024-02-jala-swap-judging

6 stars 4 forks source link

thisvishalsingh - thisvishalsingh - Permit front-running in `JalaRouter02.sol` #245

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 6 months ago

thisvishalsingh

medium

thisvishalsingh - Permit front-running in JalaRouter02.sol

thisvishalsingh

Permit front-running in JalaRouter02.sol

medium

Summary

The Use removeLiquidityWithPermit,removeLiquidityETHWithPermit and removeLiquidityETHWithPermitSupportingFeeOnTransferTokens functions of JalaRouter02 contract are designed to allow users to remove liquidity from a pair of tokens or a token and WETH pair, with the added functionality of using a permit to approve the liquidity token transfer. The fundamental issue is that frontrunner could front-run the call to permit, which would cause these functions to revert due to the liquidity being removed by the frontrunner.

My suggestion is : to wrap each call to permit in a try/catch statement.

Vulnerability Detail

  1. Frontrunning in removeLiquidityWithPermit , removeLiquidityETHWithPermit and removeLiquidityETHWithPermitSupportingFeeOnTransferTokens Functions

A frontrunner could monitor the mempool for the original transaction, see the permit call, and execute their own transaction to remove liquidity before the original transaction is confirmed. This would cause the original transaction to revert due to the liquidity being removed by the frontrunner.

Also there are other things might gone wrong:

  1. Incorrect Permit Signature If the permit signature (v, r, s) is incorrect or does not match the expected parameters, the permit call will fail, causing the transaction to revert. Implement checks to validate the signature before proceeding with the transaction.
  2. Insufficient Liquidity If the liquidity provided in the functions is insufficient, the transaction could revert.

Implement checks to ensure that the liquidity provided is sufficient for the removal operation.

Impact

Financial Loss for Users if their transactions are frontrun and will decrease in user adoption.

Code Snippet

Tool used

Manual Review

Recommendation

My suggestion is : to wrap each call to permit in a try/catch statement. And reconsider it.

Duplicate of #177