From dfa7033d2e1aa2f2845335d29d6142cc9a7ac5f1 Mon Sep 17 00:00:00 2001 From: Diana Carvalho Date: Tue, 28 Jan 2025 13:03:04 +0000 Subject: [PATCH] feat: Smother slither and add a reentrancy guard in swap() --- don't change below this line --- ENG-4041 Took 34 minutes --- foundry/src/ExecutionDispatcher.sol | 5 ++--- foundry/src/TychoRouter.sol | 13 +++++++++---- foundry/src/executors/UniswapV2Executor.sol | 1 + 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/foundry/src/ExecutionDispatcher.sol b/foundry/src/ExecutionDispatcher.sol index 8a80d74..e8b36e8 100644 --- a/foundry/src/ExecutionDispatcher.sol +++ b/foundry/src/ExecutionDispatcher.sol @@ -50,7 +50,7 @@ contract ExecutionDispatcher { * @dev Calls an executor, assumes swap.protocolData contains * protocol-specific data required by the executor. */ - // slither-disable-next-line dead-code + // slither-disable-next-line delegatecall-loop function _callExecutor( address executor, bytes4 selector, @@ -62,8 +62,7 @@ contract ExecutionDispatcher { } selector = selector == bytes4(0) ? IExecutor.swap.selector : selector; - - // slither-disable-next-line low-level-calls + // slither-disable-next-line controlled-delegatecall,low-level-calls (bool success, bytes memory result) = executor.delegatecall( abi.encodeWithSelector(selector, amount, data) ); diff --git a/foundry/src/TychoRouter.sol b/foundry/src/TychoRouter.sol index 880aa77..616d5c0 100644 --- a/foundry/src/TychoRouter.sol +++ b/foundry/src/TychoRouter.sol @@ -7,10 +7,11 @@ import "./CallbackVerificationDispatcher.sol"; import "@openzeppelin/contracts/access/AccessControl.sol"; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; +import "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; +import "@openzeppelin/contracts/utils/Pausable.sol"; import "@permit2/src/interfaces/IAllowanceTransfer.sol"; import "./ExecutionDispatcher.sol"; import "./CallbackVerificationDispatcher.sol"; -import "@openzeppelin/contracts/utils/Pausable.sol"; import {LibSwap} from "../lib/LibSwap.sol"; error TychoRouter__WithdrawalFailed(); @@ -22,7 +23,8 @@ contract TychoRouter is AccessControl, ExecutionDispatcher, CallbackVerificationDispatcher, - Pausable + Pausable, + ReentrancyGuard { IAllowanceTransfer public immutable permit2; IWETH private immutable _weth; @@ -125,7 +127,9 @@ contract TychoRouter is IAllowanceTransfer.PermitSingle calldata permitSingle, bytes calldata signature, bytes calldata swaps - ) external payable whenNotPaused returns (uint256 amountOut) { + ) external payable whenNotPaused nonReentrant returns (uint256 amountOut) { + require(receiver != address(0), "Invalid receiver address"); + // For native ETH, assume funds already in our router. Else, transfer and handle approval. if (wrapEth) { _wrapETH(amountIn); @@ -145,7 +149,7 @@ contract TychoRouter is uint256 feeAmount = (amountOut * fee) / 10000; amountOut -= feeAmount; IERC20(tokenOut).safeTransfer(feeReceiver, feeAmount); - if (unwrapEth == false) { + if (!unwrapEth) { IERC20(tokenOut).safeTransfer(receiver, amountOut); } } @@ -156,6 +160,7 @@ contract TychoRouter is if (unwrapEth) { _unwrapETH(amountOut); + // slither-disable-next-line arbitrary-send-eth payable(receiver).transfer(amountOut); } } diff --git a/foundry/src/executors/UniswapV2Executor.sol b/foundry/src/executors/UniswapV2Executor.sol index d892122..9f447f1 100644 --- a/foundry/src/executors/UniswapV2Executor.sol +++ b/foundry/src/executors/UniswapV2Executor.sol @@ -10,6 +10,7 @@ error UniswapV2Executor__InvalidDataLength(); contract UniswapV2Executor is IExecutor { using SafeERC20 for IERC20; + // slither-disable-next-line locked-ether function swap(uint256 givenAmount, bytes calldata data) external payable