From ad0748e9c3b2431ae29be8477534853029efa27d Mon Sep 17 00:00:00 2001 From: TAMARA LIPOWSKI Date: Fri, 24 Jan 2025 16:55:21 -0500 Subject: [PATCH] feat: Perform staticcall to CallbackVerifier --- foundry/interfaces/ICallbackVerifier.sol | 18 ++ .../src/CallbackVerificationDispatcher.sol | 66 ++++++- .../test/CallbackVerificationDispatcher.t.sol | 163 ++++++++++++++++++ 3 files changed, 246 insertions(+), 1 deletion(-) create mode 100644 foundry/interfaces/ICallbackVerifier.sol create mode 100644 foundry/test/CallbackVerificationDispatcher.t.sol diff --git a/foundry/interfaces/ICallbackVerifier.sol b/foundry/interfaces/ICallbackVerifier.sol new file mode 100644 index 0000000..3054132 --- /dev/null +++ b/foundry/interfaces/ICallbackVerifier.sol @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.28; + +interface ICallbackVerifier { + error UnauthorizedCaller(string exchange, address sender); + + /** + * @dev This method should revert if the sender is not a verified sender of the exchange. + */ + function verifyCallback(address sender, bytes calldata data) + external + returns ( + uint256 amountOwed, + uint256 amountReceived, + address tokenOwed, + uint16 dataOffset + ); +} diff --git a/foundry/src/CallbackVerificationDispatcher.sol b/foundry/src/CallbackVerificationDispatcher.sol index 89e05d1..072dc93 100644 --- a/foundry/src/CallbackVerificationDispatcher.sol +++ b/foundry/src/CallbackVerificationDispatcher.sol @@ -1,5 +1,10 @@ // SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.0; +pragma solidity ^0.8.28; + +import "@interfaces/ICallbackVerifier.sol"; + +error CallbackVerificationDispatcher__UnapprovedVerifier(); +error CallbackVerificationDispatcher__NonContractVerifier(); /** * @title Dispatch callback verification to external contracts @@ -12,4 +17,63 @@ pragma solidity ^0.8.0; */ contract CallbackVerificationDispatcher { mapping(address => bool) public callbackVerifiers; + + /** + * @dev Calls a callback verifier. This should revert if the callback verification fails. + */ + // slither-disable-next-line dead-code + function _callVerifyCallback(bytes calldata data) + internal + returns ( + uint256 amountOwed, + uint256 amountReceived, + address tokenOwed, + uint16 dataOffset + ) + { + address verifier; + bytes4 decodedSelector; + bytes memory verifierData; + + (verifier, decodedSelector, verifierData) = + _decodeVerifierAndSelector(data); + + if (!callbackVerifiers[verifier]) { + revert CallbackVerificationDispatcher__UnapprovedVerifier(); + } + + bytes4 selector = decodedSelector == bytes4(0) + ? ICallbackVerifier.verifyCallback.selector + : decodedSelector; + + address sender = msg.sender; + + // slither-disable-next-line low-level-calls + (bool success, bytes memory result) = verifier.staticcall( + abi.encodeWithSelector(selector, sender, verifierData) + ); + + if (!success) { + if (result.length > 0) { + revert(string(result)); + } else { + revert("Callback verification failed"); + } + } + + (amountOwed, amountReceived, tokenOwed, dataOffset) = + abi.decode(result, (uint256, uint256, address, uint16)); + } + + // slither-disable-next-line dead-code + function _decodeVerifierAndSelector(bytes calldata data) + internal + pure + returns (address verifier, bytes4 selector, bytes memory verifierData) + { + require(data.length >= 20, "Invalid data length"); + verifier = address(uint160(bytes20(data[:20]))); + selector = bytes4(data[20:24]); + verifierData = data[24:]; + } } diff --git a/foundry/test/CallbackVerificationDispatcher.t.sol b/foundry/test/CallbackVerificationDispatcher.t.sol new file mode 100644 index 0000000..7d2fcae --- /dev/null +++ b/foundry/test/CallbackVerificationDispatcher.t.sol @@ -0,0 +1,163 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.28; + +import "@src/CallbackVerificationDispatcher.sol"; +import "./TychoRouterTestSetup.sol"; + +contract CallbackVerificationDispatcherExposed is + CallbackVerificationDispatcher +{ + function exposedCallVerifier(bytes calldata data) + external + returns ( + uint256 amountOwed, + uint256 amountReceived, + address tokenOwed, + uint16 dataOffset + ) + { + return _callVerifyCallback(data); + } + + function exposedDecodeVerifierAndSelector(bytes calldata data) + external + pure + returns (address executor, bytes4 selector, bytes memory protocolData) + { + return _decodeVerifierAndSelector(data); + } + + function exposedSetVerifier(address target) external { + callbackVerifiers[target] = true; + } + + function exposedRemoveVerifier(address target) external {} +} + +contract CallbackVerificationDispatcherTest is Constants { + CallbackVerificationDispatcherExposed dispatcherExposed; + + event VerifierSet(address indexed executor); + event VerifierRemoved(address indexed executor); + + function setUp() public { + uint256 forkBlock = 20673900; + vm.createSelectFork(vm.rpcUrl("mainnet"), forkBlock); + dispatcherExposed = new CallbackVerificationDispatcherExposed(); + deal(WETH_ADDR, address(dispatcherExposed), 15 ether); + deployDummyContract(); + } + + function testCallVerifierSuccess() public { + // For this test, we can use any callback verifier and any calldata that we + // know works for this verifier. We don't care about which calldata/executor, + // since we are only testing the functionality of the staticcall and not + // the inner verifier. + // Thus, this test case designed from scratch using previously-deployed + // Maverick callback verifier. Looking at the code, we can easily design + // passing calldata. + dispatcherExposed.exposedSetVerifier( + address(0x2C960bD1CFE09A26105ad3C351bEa0a3fAD0F8e8) + ); + bytes memory data = + hex"2C960bD1CFE09A26105ad3C351bEa0a3fAD0F8e876b20f8a0000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000001A0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48"; + vm.startPrank(address(0xD0b2F5018B5D22759724af6d4281AC0B13266360)); + ( + uint256 amountOwed, + uint256 amountReceived, + address tokenOwed, + uint16 dataOffset + ) = dispatcherExposed.exposedCallVerifier(data); + vm.stopPrank(); + + // The values themselves are irrelevant, we just need to make sure that we + // correctly parse the expected output of the existing Maverick verifier + assert(amountOwed == 1); + assert(amountReceived == 1); + assert(tokenOwed == address(0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48)); + assert(dataOffset == 148); + } + + function testCallVerifierNoSelector() public { + // For this test, we can use any callback verifier and any calldata that we + // know works for this verifier. We don't care about which calldata/executor, + // since we are only testing the functionality of the staticcall and not + // the inner verifier. + // Thus, this test case designed from scratch using previously-deployed + // Maverick callback verifier. Looking at the code, we can easily design + // passing calldata. + dispatcherExposed.exposedSetVerifier( + address(0x2C960bD1CFE09A26105ad3C351bEa0a3fAD0F8e8) + ); + + // Pass all-zero selector. This should default to the verifyCallback selector + bytes memory data = + hex"2C960bD1CFE09A26105ad3C351bEa0a3fAD0F8e8000000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000001A0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48"; + vm.startPrank(address(0xD0b2F5018B5D22759724af6d4281AC0B13266360)); + ( + uint256 amountOwed, + uint256 amountReceived, + address tokenOwed, + uint16 dataOffset + ) = dispatcherExposed.exposedCallVerifier(data); + vm.stopPrank(); + + // The values themselves are irrelevant, we just need to make sure that we + // correctly parse the expected output of the existing Maverick verifier + assert(amountOwed == 1); + assert(amountReceived == 1); + assert(tokenOwed == address(0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48)); + assert(dataOffset == 148); + } + + function testCallVerifierBadSelector() public { + // A bad selector is provided to an approved executor - causing the call + // itself to fail. Make sure this actually reverts. + dispatcherExposed.exposedSetVerifier( + address(0x2C960bD1CFE09A26105ad3C351bEa0a3fAD0F8e8) + ); + vm.startPrank(address(0xD0b2F5018B5D22759724af6d4281AC0B13266360)); + bytes memory data = + hex"2C960bD1CFE09A26105ad3C351bEa0a3fAD0F8e8aa0000000000"; + vm.expectRevert(bytes("Callback verification failed")); + dispatcherExposed.exposedCallVerifier(data); + vm.stopPrank(); + } + + function testCallVerifierParseRevertMessage() public { + // Verification should fail because caller is not a Maverick pool + // Check that we correctly parse the revert message + dispatcherExposed.exposedSetVerifier( + address(0x2C960bD1CFE09A26105ad3C351bEa0a3fAD0F8e8) + ); + bytes memory data = + hex"2C960bD1CFE09A26105ad3C351bEa0a3fAD0F8e8000000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000001A0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48"; + vm.expectRevert( + abi.encodeWithSignature( + "Error(string)", "Must call from a Maverick Factory Pool" + ) + ); + dispatcherExposed.exposedCallVerifier(data); + } + + function testCallVerifierUnapprovedVerifier() public { + bytes memory data = + hex"5d622C9053b8FFB1B3465495C8a42E603632bA70aabbccdd1111111111111111"; + vm.expectRevert(); + dispatcherExposed.exposedCallVerifier(data); + } + + function testDecodeVerifierAndSelector() public { + bytes memory data = + hex"2C960bD1CFE09A26105ad3C351bEa0a3fAD0F8e876b20f8aA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48"; + (address executor, bytes4 selector, bytes memory verifierData) = + dispatcherExposed.exposedDecodeVerifierAndSelector(data); + assert(executor == address(0x2C960bD1CFE09A26105ad3C351bEa0a3fAD0F8e8)); + assert(selector == bytes4(0x76b20f8a)); + // Direct bytes comparison not supported - must use keccak + assert( + keccak256(verifierData) + == keccak256(hex"A0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48") + ); + } +}