ak1 - DODORouteProxy.sol : The `receiveAmount` from all the swap is not the actual value. It is deducted from all the fee.

[github-actions[bot]]

ak1

high

DODORouteProxy.sol : The receiveAmount from all the swap is not the actual value. It is deducted from all the fee.

Summary

During the swap operations, externalSwap or ` ormixSwapordodoMutliSwap. thereceiveAmount` is returned and the same is emitted. But the emitted/returned amount value is incorrect as it undergone for fee deduction.

Vulnerability Detail

When look at the external swap https://github.com/sherlock-audit/2022-11-dodo/blob/main/contracts/SmartRoute/DODORouteProxy.sol#L174

https://github.com/sherlock-audit/2022-11-dodo/blob/main/contracts/SmartRoute/DODORouteProxy.sol#L228

Inside the _routeWithdraw function, the fee are deducted and remaining value is sent to msg.sender. https://github.com/sherlock-audit/2022-11-dodo/blob/main/contracts/SmartRoute/DODORouteProxy.sol#L465-L492

The value that is sent to user and emitted or returned from swap functions are not same.

Impact

The information about the receiveAmount is not correct. User could be given with incorrect data.

Code Snippet

For external swap. https://github.com/sherlock-audit/2022-11-dodo/blob/main/contracts/SmartRoute/DODORouteProxy.sol#L164-L174 https://github.com/sherlock-audit/2022-11-dodo/blob/main/contracts/SmartRoute/DODORouteProxy.sol#L228 Similarly we can see for other swaps also.

Tool used

Manual Review

Recommendation

return the receiveAmount value from _routeWithdraw where it has undergone for all type of fee deduction. and use this value to emit and return in all the swaps.

[aktech297]

Escalate for 2 USDC I believe this is valid issue. Conveying the incorrect data will not be suitable for both end user as well as for the protocol. Any front end operation that rely on the emitted receiveAmount value will work with incorrect data. I have seen the issues where absence of emit event is considered as medium. Refer the bond protocol judgement here. When absence of emit event is considered as medium, I believe this one also will qualify for a valid issue. This issue is there in all type of swap operations. Refer the code snippet section of the issue.

[sherlock-admin]

Escalate for 2 USDC I believe this is valid issue. Conveying the incorrect data will not be suitable for both end user as well as for the protocol. Any front end operation that rely on the emitted receiveAmount value will work with incorrect data. I have seen the issues where absence of emit event is considered as medium. Refer the bond protocol judgement here. When absence of emit event is considered as medium, I believe this one also will qualify for a valid issue. This issue is there in all type of swap operations. Refer the code snippet section of the issue.

You've created a valid escalation for 2 USDC!

To remove the escalation from consideration: Delete your comment. To change the amount you've staked on this escalation: Edit your comment (do not create a new comment).

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[Evert0x]

Escalation rejected.

In general we don't consider these missing or invalid events medium or high severity. The bond judgment has not been finalized.

[sherlock-admin]

Escalation rejected.

In general we don't consider these missing or invalid events medium or high severity. The bond judgment has not been finalized.

This issue's escalations have been rejected!

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

[aktech297]

imo , it's not about event alone, based on the emitted value there would be some operations done is front end. Also, when we look at all the swap functions they are returning the incorrect value. Again any operation done based on the incorrect data could harm to the protocol in numerous way. I think it will be medium or high.. depends on the situations.. it's dark area and can cause any issue.. I am seeing in many reports that absence of event is treated as either high or medium based on the situations..in our case , event emit and function return with incorrect data. We have event in order to capture the updates that happen on chain and process in the front end with less gas consumption... I believe dodo has the event to do similar operation in the front end