Caller can steal funds from the swapper flash function by reentrancy
Summary
flash function in swapper contract uses the callback on msg.sender, which can be used by msg.sender to reenter and steal the funds.
Vulnerability Detail
Suppose there are 10DAI, 10 OP and 10 ARB tokens in the swapper contract, and beneficiary token is lets say LINK.
First the _transferToTrader happens, lets say user swaps 1 dai, 1 op and 1 arb for 1 Link, we assuming that amount for the beneficiary comes to be 1LINK from the transferToTrader function.
Next the msg.sender reenters through the callback function.
Reenter and repeat the step 2 until all the tokens are transferred to trader. But in memory the value returned by the transferToTrader is still returned as 1LINK but in actuality it should have been 10 LINK tokens which is for the last swap only.
There can be many other alternatives to drain for example caller may first get the 90% tokens and than reenter to get 10%tokens and only give one LINK to beneficiary as transfer to beneficiary happens after the callback, even worse may give 0.01% only dust.
0xnirlin
high
Caller can steal funds from the swapper
flashfunction by reentrancySummary
flashfunction in swapper contract uses the callback on msg.sender, which can be used by msg.sender to reenter and steal the funds.Vulnerability Detail
_transferToTraderhappens, lets say user swaps 1 dai, 1 op and 1 arb for 1 Link, we assuming that amount for the beneficiary comes to be 1LINK from thetransferToTraderfunction.transferToTraderis still returned as 1LINK but in actuality it should have been 10 LINK tokens which is for the last swap only.There can be many other alternatives to drain for example caller may first get the 90% tokens and than reenter to get 10%tokens and only give one LINK to beneficiary as transfer to beneficiary happens after the callback, even worse may give 0.01% only dust.
Impact
User funds can be stolen
Code Snippet
https://github.com/sherlock-audit/2023-04-splits/blob/main/splits-swapper/src/SwapperImpl.sol#L203-L221
Tool used
Manual Review
Recommendation
Callback mechanism is necessary for ethereum deposits through payback. But a reentrancy guard can be applied to prevent the reentrancy.
Duplicate of #49