c7e7eff - Trader is overpaid when base token is the same as quote token.

[sherlock-admin]

c7e7eff

medium

Trader is overpaid when base token is the same as quote token.

c7e7eff Medium

Summary

The UniV3OracleImpl oracle has a $defaultScaledOfferFactor storage variable indicating the discount the owner of the Swapper contract is willing to give a trader for the service of swapping undesired tokens in to the token desired by the beneficiary. The discount percentage can be set to any value, but in the tests a typical discount of 0.1% to 2% (scaledOfferFactorof 99.9% and 98%) is used hence we can assume these values are typical. When a Swapper holds any amount of funds in the beneficiary token a trader can us the flashfunction to get the total amount of those tokens out and only repay the discounted amount. As in this case no swap is necessary this is obviously overpaid and can be seen as loss of funds.

Vulnerability Detail

The _getQuoteAmount function in the UniV3OracleImpl contract specifically treats the base and quote tokens being equal as a separate case and does not call the Uniswap oracle for price information and just applies the scaledOfferFactor.

    function _getQuoteAmount(QuoteParams calldata quoteParams_) internal view returns (uint256) {
    ...
    // skip oracle if converted tokens are equal
    if (cqp.cBase == cqp.cQuote) {
        return quoteParams_.baseAmount * po.scaledOfferFactor / PERCENTAGE_SCALE;
    }

There is no mention of this special case in the documentation or contest description that the owner of the swapper should take this into consideration. Furthermore at the end of the flash function when the beneficiary tokens payed back by the trader are sent to the beneficiary, the excess amount (difference between contracts balance and the amount payed back by the trader) is sent to the beneficiary anyway. So it seems unnecessary to allow a trader to get any type of discount on those tokens.

Impact

Any trader can claim/steal a percentage designated by the $defaultScaledOfferFactor of the beneficiary tokens in the swapper contract.

Code Snippet

https://github.com/0xSplits/splits-oracle/blob/f6628a116d8721289dad2c70d3e3aa14e4815d4e/src/UniV3OracleImpl.sol#L259µ https://github.com/0xSplits/splits-swapper/blob/83ce1124767a097aac37d1cd162a9b27bbc48701/src/SwapperImpl.sol#L278

Tool used

Manual Review

Recommendation

Disallow a trader to call flash for the beneficiary designated token and let them automatically be sent to the beneficiary as currently is coded in the _transferToBeneficiary function.

Duplicate of #104