sherlock-audit / 2023-04-splits-judging

4 stars 1 forks source link

ck - Beneficiary can incur losses due to slippage during a flash call #109

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ck

medium

Beneficiary can incur losses due to slippage during a flash call

Summary

Beneficiary can incur losses due to slippage during a flash call

Vulnerability Detail

When SwapperImpl::flash is called, the amountsToBeneficiary is determined by the oracle using the getQuoteAmounts function.

        amountsToBeneficiary = $oracle.getQuoteAmounts(quoteParams_);
        uint256 length = quoteParams_.length;
        if (amountsToBeneficiary.length != length) revert Invalid_AmountsToBeneficiary();

The issue is that there is no slippage protection to ensure that amountsToBeneficiary is within a certain minimum limit.

This exposes the calculation of the amountsToBeneficiary to attacks including flashloans where the trader gains an advantage and the beneficiary loses out by getting a lower value of amountsToBeneficiary.

Impact

Beneficiary loses value due to lack of slippage protection.

Code Snippet

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

https://github.com/sherlock-audit/2023-04-splits/blob/main/splits-swapper/src/integrations/UniV3Swap.sol#L78-L88

Tool used

Manual Review

Recommendation

Some form of slippage protection should be implemented which may require a redesign of how trades are done.

Duplicate of #37