The Standard

The Standard
DeFiHardhat
20,000 USDC
View results
Submission Details
Severity: medium
Valid

Protocol wrongly assumes all Uniswap pool exist with the same fee

Proof of Concept

Take a look at SmartVaultV3.sol#L214-L231

function swap(bytes32 _inToken, bytes32 _outToken, uint256 _amount) external onlyOwner {
uint256 swapFee = _amount * ISmartVaultManagerV3(manager).swapFeeRate() / ISmartVaultManagerV3(manager).HUNDRED_PC();
address inToken = getSwapAddressFor(_inToken);
uint256 minimumAmountOut = calculateMinimumAmountOut(_inToken, _outToken, _amount);
ISwapRouter.ExactInputSingleParams memory params = ISwapRouter.ExactInputSingleParams({
tokenIn: inToken,
tokenOut: getSwapAddressFor(_outToken),
//@audit
fee: 3000,
recipient: address(this),
deadline: block.timestamp,
amountIn: _amount - swapFee,
amountOutMinimum: minimumAmountOut,
sqrtPriceLimitX96: 0
});
inToken == ISmartVaultManagerV3(manager).weth() ?
executeNativeSwapAndFee(params, swapFee) :
executeERC20SwapAndFee(params, swapFee);
}

As seen, coupled with this section of the Uniswap docs, we can understand that the intended functionality of this is that no matter what the tokenIn or tokenOut is, the protocol assumes that not only does a pool for this pair exist with the fee, i.e 0.3% but also assumes that it's the most optimal pool.

To dive a bit deeper into this, from here we can that using the ETH has 3 different pool against USDC, with the two most optimal() being with a fee other than protcols currently hardcoded value of 0.3%, note that this is even a pair of two popular tokens, same case can be made for the WBTC/ETH pair, where it's best pool with ~23 mln TVL is the pool with an attached fee of 0.05% and the pool with a 0.3% also existing but having a TVL of 14% in comparison to the latter now if these are the massive differences for popular tokens the pair of the popular ETH and any other

Impact

There are multiple cases to be made from this, one in an instance where this pool(i.e with fee 0.3%) does not exist for this pair, then the whole attempt to swap reverts, causing a permanent DOS to swapping these pairs, in a case where this pool exists but the liquidity is not optimal, then attempts to swap could still revert when not enough tokens are available to cover calculateMinimumAmountOut

Recommended Mitigation Steps

Each swap amount, should be accompanied with the needed fee so a provision can be made for the specific pool to use

Additional Note

A bit related to this report is also the issue of directly routing tokenIn into tokenOut in some cases this also isn't the best option, say a swap of ARB to LINK is wanted, it might be most optimal to route the call via ARB -> WETH -> LINK or what not, and this should also be taken into consideration and allowed to be specified in each attempt of swapping.

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

fixed-uni-fee

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

hardcoded-fee

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.