simon135 - An attacker can reenter `flash` by using the `ISwapperFlashCallback` callback

[sherlock-admin]

simon135

high

An attacker can reenter flash by using the ISwapperFlashCallback callback

Summary

After the attacker gets erc20 tokens from the swapper they can reenter flash and get more erc20 tokens

Vulnerability Detail

Since the flash callback is called by msg.sender since _transferToTrader gives the caller erc20/eth and then the callback is called.So the caller will get more tokens than spent. ex: Beneficiary = bob Alice = attacker 100 usdc and 100 dai and swapper:dai the user provides 100 dai and they should get back 100 usdc but since of the malicious callback, the user doesn't have to provide any amount of tokens for swapping to dai.

Alice is a smart contract that reenters at the end, when all the funds are gone and the last reenter supplies 0 params, they should end the tx with all the funds in the contract and no loss.

Impact

be Beneficiary loses their funds

Code Snippet

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

 address _tokenToBeneficiary = $tokenToBeneficiary;
        (uint256 amountToBeneficiary, uint256[] memory amountsToBeneficiary) =
            _transferToTrader(_tokenToBeneficiary, quoteParams_);

        ISwapperFlashCallback(msg.sender).swapperFlashCallback({
            tokenToBeneficiary: _tokenToBeneficiary,
            amountToBeneficiary: amountToBeneficiary,
            data: callbackData_
        });

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

            if (amountToTrader > tokenToTrader._balanceOf(address(this))) {
                revert InsufficientFunds_InContract();
            }

            amountToBeneficiary += amountsToBeneficiary[i];
            tokenToTrader._safeTransfer(msg.sender, amountToTrader);

            unchecked {
                ++i;
            }

Tool used

VIM Manual Review

Recommendation

add a non-reentrant modifier and use the checks and effects pattern

Duplicate of #49