Swap lacks slippage and path checks
Description
The Uniswap
↗ module of swapping collateral
into JRT
does not support passing a parameter for the slippage check.
amountOut = router.swapExactTokensForTokens(
amountIn,
0, // no slippage check
swapInfo.tokenSwapPath,
recipient,
swapInfo.expiration
)[swapInfo.tokenSwapPath.length - 1];
Moreover, the last element of the swap's path is not checked to be the JRT
token.
Impact
The protocol may lose tokens due to overallowance of slippage, since the swap
itself can get sandwich attacked by front runners. This may heavily affect larger amounts of collateral being swapped.
Recommendations
We recommend implementing the minTokensOut
field in the SwapInfo
and then passing that in the swap
function call.
amountOut = router.swapExactTokensForTokens(
amountIn,
swapInfo.minTokensOut, // slippage check passed
swapInfo.tokenSwapPath,
recipient,
swapInfo.expiration
)[swapInfo.tokenSwapPath.length - 1];
Moreover, similarly to the BalancerJRTSwap
's SwapInfo
struct, we recommend adding the jrtAddress
field and checking it to match with the last index of the swap
path, like so:
...
uint256 swapLength = swapInfo.tokenSwapPath.length;
require(
swapInfo.tokenSwapPath[swapLength - 1] == jrtAddress,
'Wrong swap asset'
);
...
Remediation
Jarvis has sufficiently addressed the finding by introducing the necessary anti-slippage parameter and required check for the last element of the swap path to be equal to the address of the JRT token (e1713b3b31b3fd71b41af84b8ed488bf998714e8↗).