simon135 - when `direction=1` the direction on pools could be wrong

[github-actions[bot]]

simon135

medium

when direction=1 the direction on pools could be wrong

Summary

since we are changing direction=1 when we change the value it will only sellBase() not sellQuote. So if direction=1 then In the first iteration of the loop it's going to sell quote tokens but after that it's going to sell Base tokens which I don't think it intended

Vulnerability Detail

when a user calls mixSwap function and sets directions =1 in the first iteration of the loop directions gets changed to zero and once it's set to zero sellBase function is always called.

Impact

It can cause loss of funds in the most extreme cases but I don't think intended and can make a user swap lost funds because they didn't sell quote tokens. In the comments, it says this

/// @param directions pool directions aggregation, one bit represents one pool direction, 0 means sellBase, 1 means sellQuote

which is not true because 1 in the loop with bit shifting will be different.

Code Snippet

        if (directions & 1 == 0) {
                IDODOAdapter(mixAdapters[i]).sellBase(
                    assetTo[i + 1],
                    mixPairs[i],
                    moreInfos[i]
                );
            } else {
                IDODOAdapter(mixAdapters[i]).sellQuote(
                    assetTo[i + 1],
                    mixPairs[i],
                    moreInfos[i]
            }
            directions = directions >> 1;
        }

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

Tool used

Manual Review

Recommendation

maybe make some sort of offset every iteration of the loop to make sellQuote and sellBase functions switch off or have an extra pram like:

pseudocode:
directions=5 then the sell base is always called
directions=2 then the sell quote is always called 

Or just have in the UI the calculation for bigger numbers what would happen in the loop and tell the user instead of letting random input which can cause loss of funds.

[Attens1423]

This is the routeProxy suitable for our own aggregator and we expect users use right parameter to avoid gas wasting