J4de - `SwapperImpl.sol#_transferToTrader` price may not match expectations

[sherlock-admin]

J4de

high

SwapperImpl.sol#_transferToTrader price may not match expectations

Summary

SwapperImpl.sol#_transferToTrader price may not match expectations

Vulnerability Detail

File: splits-swapper/src/SwapperImpl.sol
227     function _transferToTrader(address tokenToBeneficiary_, OracleImpl.QuoteParams[] calldata quoteParams_)
228         internal
229         returns (uint256 amountToBeneficiary, uint256[] memory amountsToBeneficiary)
230     {
231 1>      amountsToBeneficiary = $oracle.getQuoteAmounts(quoteParams_);
232         uint256 length = quoteParams_.length;
233         if (amountsToBeneficiary.length != length) revert Invalid_AmountsToBeneficiary();

SwapperImpl.sol contract uses discount oracle pricing to incentivize third parties to automatically convert multi-token revenue into a single token & forward to beneficiary. The price of the user's exchange tokens is calculated by $oracle.

File: splits-oracle/src/UniV3OracleImpl.sol
248     function _getQuoteAmount(QuoteParams calldata quoteParams_) internal view returns (uint256) {
249         ConvertedQuotePair memory cqp = quoteParams_.quotePair._convert(_convertToken);
250 1>      SortedConvertedQuotePair memory scqp = cqp._sort();
251
252         PairOverride memory po = _getPairOverride(scqp);
253         if (po.scaledOfferFactor == 0) {
254             po.scaledOfferFactor = $defaultScaledOfferFactor;
255         }

The _getQuoteAmount function will sort the addresses of the two tokens from small to large.

File: splits-swapper/src/SwapperImpl.sol
227     function _transferToTrader(address tokenToBeneficiary_, OracleImpl.QuoteParams[] calldata quoteParams_)
228         internal
229         returns (uint256 amountToBeneficiary, uint256[] memory amountsToBeneficiary)
230     {
231         amountsToBeneficiary = $oracle.getQuoteAmounts(quoteParams_);
232         uint256 length = quoteParams_.length;
233         if (amountsToBeneficiary.length != length) revert Invalid_AmountsToBeneficiary();
234
235         uint128 amountToTrader;
236         address tokenToTrader;
237         for (uint256 i; i < length;) {
238  1>         OracleImpl.QuoteParams calldata qp = quoteParams_[i];
239
240             if (tokenToBeneficiary_ != qp.quotePair.quote) revert Invalid_QuoteToken();
241  2>         tokenToTrader = qp.quotePair.base;
242             amountToTrader = qp.baseAmount;
243
244             if (amountToTrader > tokenToTrader._balanceOf(address(this))) {
245                 revert InsufficientFunds_InContract();
246             }
247
248             amountToBeneficiary += amountsToBeneficiary[i];
249   3>        tokenToTrader._safeTransfer(msg.sender, amountToTrader);
250
251             unchecked {
252                 ++i;
253             }
254         }
255     }

But in the _transferToTrader function, directly use quoteParams_.quotePair.base as tokenToTrader (the base and queto are not sorted). In some cases, the token pair used to calculate the price and the actual transfer is opposite, and the attacker can use this to steal the funds in the contract.

For example,

1. Suppose there are TokenA and TokenB, their addresses are 0xAAAA and 0xBBBB respectively, and their values are 100 USDC and 1 USDC respectively

2. The attacker packs them into a QuoteParams and passes them to the flash function

   struct QuoteParams {
      QuotePair quotePair; // { .base = 0xBBBB, .quote = 0xAAAA }
      uint128 baseAmount;  // 1000
      bytes data;
   }

3. $oracle.getQuoteAmounts will sort the tokens when calculating the price, and the obtained quantity is

   // SortedConvertedQuotePair { .cToken0 = 0xAAAA, .cToken1 = 0xBBBB }
   amountsToBeneficiary = 1000 * 1 USDC / 100 USDC = 10

4. Then, the attacker can obtain 1000 TokenB (worth 100,000 USDC) and consume 10 TokenA (worth 10 USDC)

Impact

Attackers can steal funds inside the contract

Code Snippet

https://github.com/0xSplits/splits-swapper/blob/83ce1124767a097aac37d1cd162a9b27bbc48701/src/SwapperImpl.sol#L227-L255

Tool used

Manual Review

Recommendation

It is recommended that the flash function transfer also sort the tokens to maintain consistency with the calculated price