From 8969186654ec5db96bdcaab0099b883f1ed0ba63 Mon Sep 17 00:00:00 2001 From: TAMARA LIPOWSKI Date: Thu, 13 Mar 2025 12:19:55 -0400 Subject: [PATCH] feat: allow to pass msg.sender to USV3 callback - So that we can possibly do a transferFrom - This should still be safe since the user can't control what is passed here --- foundry/src/executors/ExecutorTransferMethods.sol | 3 ++- foundry/src/executors/UniswapV2Executor.sol | 2 +- foundry/src/executors/UniswapV3Executor.sol | 8 +++++--- foundry/test/executors/UniswapV3Executor.t.sol | 3 ++- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/foundry/src/executors/ExecutorTransferMethods.sol b/foundry/src/executors/ExecutorTransferMethods.sol index 45fc1d2..9736994 100644 --- a/foundry/src/executors/ExecutorTransferMethods.sol +++ b/foundry/src/executors/ExecutorTransferMethods.sol @@ -29,6 +29,7 @@ contract ExecutorTransferMethods { function _transfer( IERC20 tokenIn, + address sender, address receiver, uint256 amount, TransferMethod method @@ -40,7 +41,7 @@ contract ExecutorTransferMethods { } else if (method == TransferMethod.TRANSFERPERMIT2) { // Permit2.permit is called from the TychoRouter permit2.transferFrom( - msg.sender, + sender, receiver, // Does this work if receiver is not address(this)? uint160(amount), address(tokenIn) diff --git a/foundry/src/executors/UniswapV2Executor.sol b/foundry/src/executors/UniswapV2Executor.sol index 29544ab..b54ecf7 100644 --- a/foundry/src/executors/UniswapV2Executor.sol +++ b/foundry/src/executors/UniswapV2Executor.sol @@ -49,7 +49,7 @@ contract UniswapV2Executor is IExecutor, ExecutorTransferMethods { _verifyPairAddress(target); calculatedAmount = _getAmountOut(target, givenAmount, zeroForOne); - _transfer(tokenIn, target, givenAmount, method); + _transfer(tokenIn, msg.sender, target, givenAmount, method); IUniswapV2Pair pool = IUniswapV2Pair(target); if (zeroForOne) { diff --git a/foundry/src/executors/UniswapV3Executor.sol b/foundry/src/executors/UniswapV3Executor.sol index e243dc9..09521a9 100644 --- a/foundry/src/executors/UniswapV3Executor.sol +++ b/foundry/src/executors/UniswapV3Executor.sol @@ -102,14 +102,14 @@ contract UniswapV3Executor is IExecutor, ICallback, ExecutorTransferMethods { "InvalidTransferMethod" ); TransferMethod method = TransferMethod(uint8(msgData[171])); + address sender = address(bytes20(msgData[172:192])); verifyCallback(msgData[132:]); uint256 amountOwed = amount0Delta > 0 ? uint256(amount0Delta) : uint256(amount1Delta); - // TODO This must never be a safeTransfer. Figure out how to ensure this. - _transfer(IERC20(tokenIn), msg.sender, amountOwed, method); + _transfer(IERC20(tokenIn), sender, msg.sender, amountOwed, method); return abi.encode(amountOwed, tokenIn); } @@ -167,7 +167,9 @@ contract UniswapV3Executor is IExecutor, ICallback, ExecutorTransferMethods { uint24 fee, TransferMethod method ) internal pure returns (bytes memory) { - return abi.encodePacked(tokenIn, tokenOut, fee, uint8(method), self); + return abi.encodePacked( + tokenIn, tokenOut, fee, uint8(method), msg.sender, self + ); } function _verifyPairAddress( diff --git a/foundry/test/executors/UniswapV3Executor.t.sol b/foundry/test/executors/UniswapV3Executor.t.sol index 2033edf..0b07237 100644 --- a/foundry/test/executors/UniswapV3Executor.t.sol +++ b/foundry/test/executors/UniswapV3Executor.t.sol @@ -123,7 +123,8 @@ contract UniswapV3ExecutorTest is Test, Constants { dataOffset, dataLength, protocolData, - uint8(ExecutorTransferMethods.TransferMethod.TRANSFER) + uint8(ExecutorTransferMethods.TransferMethod.TRANSFER), + address(uniswapV3Exposed) // transferFrom sender (irrelevant in this case) ); uniswapV3Exposed.handleCallback(callbackData); vm.stopPrank();