sherlock-audit / 2023-04-splits-judging

4 stars 1 forks source link

0xnirlin - All the tokens can never be swapped and leads to unexpected reverts and missed reward for caller. #140

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

0xnirlin

medium

All the tokens can never be swapped and leads to unexpected reverts and missed reward for caller.

Summary

Caller cannot swap the exact amounts, for example if swapper have 10 DAI and beneficiary token is OP, so if caller have enough token to get all 10 DAI the swap cannot happen.

Vulnerability Detail

  1. Suppose BOB is caller and alice is the benificary.
  2. Beneficiary token is OP, Swapper have the 10 Dai token and bob have enough tokens to get all the dai.
  3. Bob calls the flash, but the transaction reverts. The reason bob cannot get the 10 DAI is because in actual bob reward is 10 DAI + scaling factor and balance of contract is only 10DAI. https://github.com/sherlock-audit/2023-04-splits/blob/main/splits-swapper/src/SwapperImpl.sol#L244-L246 If user decides to swap less, there is missed reward for caller but excess funds are flushed anyways.

    Impact

    Unexpected reverts and missed reward for the caller.

    Code Snippet

    https://github.com/sherlock-audit/2023-04-splits/blob/main/splits-swapper/src/SwapperImpl.sol#L244-L246

    Tool used

Manual Review

Recommendation

Use some different approach for reward mechanism where the caller may not feel betrayed of his fair share from the system.

0xnirlin commented 1 year ago

Escalate for 10 USDC I think this is a valid medium as it always happens when trader can swap exact amount of tokens in the contract, as traders are never made aware of such risk and also trader can not do the multiple swaps as in first flash call all the excess erc20 tokens are flushed. Either make trader aware of such risk, or devise a some other incentive in such cases. Cause there is certainly missed reward when user want to swap exact amount in contract and cannot due to the check here: https://github.com/sherlock-audit/2023-04-splits/blob/main/splits-swapper/src/SwapperImpl.sol#L244-L246

sherlock-admin commented 1 year ago

Escalate for 10 USDC I think this is a valid medium as it always happens when trader can swap exact amount of tokens in the contract, as traders are never made aware of such risk and also trader can not do the multiple swaps as in first flash call all the excess erc20 tokens are flushed. Either make trader aware of such risk, or devise a some other incentive in such cases. Cause there is certainly missed reward when user want to swap exact amount in contract and cannot due to the check here: https://github.com/sherlock-audit/2023-04-splits/blob/main/splits-swapper/src/SwapperImpl.sol#L244-L246

You've created a valid escalation for 10 USDC!

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.

hrishibhat commented 1 year ago

Escalation rejected

Lead Judge comment:

All the tokens can never be swapped and leads to unexpected reverts and missed reward for caller

Lead Watson comment:

The system works as intended. The scaling factor is subtracted from what is owed, not added to what is withdrawn.

sherlock-admin commented 1 year ago

Escalation rejected

Lead Judge comment:

All the tokens can never be swapped and leads to unexpected reverts and missed reward for caller Lead Watson comment: The system works as intended. The scaling factor is subtracted from what is owed, not added to what is withdrawn.

This issue's escalations have been rejected!

Watsons who escalated this issue will have their escalation amount deducted from their next payout.