From 9d101b073c61b64d0e48dac07cf31d782569b0ef Mon Sep 17 00:00:00 2001 From: Rangel Stoilov Date: Mon, 30 Jan 2023 21:00:12 +0200 Subject: [PATCH 1/5] feat: decode simulation with a delegate call --- contracts/modules/Exec.sol | 27 ++++++++++++++++++++++++++- test/lib/eTestLib.js | 7 +------ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/contracts/modules/Exec.sol b/contracts/modules/Exec.sol index dbc3a40e..ac5706b9 100644 --- a/contracts/modules/Exec.sol +++ b/contracts/modules/Exec.sol @@ -7,7 +7,7 @@ import "../IRiskManager.sol"; import "../PToken.sol"; import "../Interfaces.sol"; import "../Utils.sol"; - +import "hardhat/console.sol"; /// @notice Definition of callback method that deferLiquidityCheck will invoke on your contract interface IDeferredLiquidityCheck { @@ -117,6 +117,31 @@ contract Exec is BaseLogic { revert("e/batch/simulation-did-not-revert"); } + /// @notice Call batch dispatch and catch the revert and parse it to EulerBatchItemResponse[] + /// @param items List of operations to execute + /// @param deferLiquidityChecks List of user accounts to defer liquidity checks for + /// @dev During simulation all batch items are executed, regardless of the `allowError` flag + /// we revert the bytes if the length is 132, otherwise we decode the bytes to EulerBatchItemResponse[] + /// length of 132 is equal to the length of the revert message "e/batch/simulation-did-not-revert" + function batchDispatchSimulateDecoded(EulerBatchItem[] calldata items, address[] calldata deferLiquidityChecks) external reentrantOK returns (EulerBatchItemResponse[] memory simulation) { + address msgSender = unpackTrailingParamMsgSender(); + bytes memory data = abi.encodeWithSelector(Exec.batchDispatchSimulate.selector, items, deferLiquidityChecks); + bytes memory inputWrapped = abi.encodePacked(data, uint160(msgSender), uint160(msg.sender)); // msg.sender is the proxy address + (bool success, bytes memory reason) = moduleLookup[MODULEID__EXEC].delegatecall(inputWrapped); + if (!success) { + if(reason.length == 132) { + revertBytes(reason); + } + assembly { + reason := add(reason, 0x04) + } + simulation = abi.decode( + reason, + (EulerBatchItemResponse[]) + ); + } + } + // Average liquidity tracking diff --git a/test/lib/eTestLib.js b/test/lib/eTestLib.js index 4c90ae81..7d6a8f87 100644 --- a/test/lib/eTestLib.js +++ b/test/lib/eTestLib.js @@ -1455,12 +1455,7 @@ class TestSet { let result; if (action.simulate) { - try { - await ctx.contracts.exec.connect(from).callStatic.batchDispatchSimulate(items, action.deferLiquidityChecks || []); - } catch (e) { - if (e.errorName !== 'BatchDispatchSimulation') throw e; - result = e.errorArgs.simulation; - } + result = await ctx.contracts.exec.connect(from).callStatic.batchDispatchSimulateDecoded(items, action.deferLiquidityChecks || []); } else { let tx = await ctx.contracts.exec.connect(from).batchDispatch(items, action.deferLiquidityChecks || []); result = await tx.wait(); From 2e175d3b1bbb2994fd2b5cd1e141a41d1d01a15c Mon Sep 17 00:00:00 2001 From: Rangel Stoilov Date: Tue, 31 Jan 2023 12:42:49 +0200 Subject: [PATCH 2/5] refactor: :fire: remove hardhat console sol --- contracts/modules/Exec.sol | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/contracts/modules/Exec.sol b/contracts/modules/Exec.sol index ac5706b9..bd468bde 100644 --- a/contracts/modules/Exec.sol +++ b/contracts/modules/Exec.sol @@ -7,7 +7,6 @@ import "../IRiskManager.sol"; import "../PToken.sol"; import "../Interfaces.sol"; import "../Utils.sol"; -import "hardhat/console.sol"; /// @notice Definition of callback method that deferLiquidityCheck will invoke on your contract interface IDeferredLiquidityCheck { @@ -133,7 +132,7 @@ contract Exec is BaseLogic { revertBytes(reason); } assembly { - reason := add(reason, 0x04) + reason := add(reason, 0x4) } simulation = abi.decode( reason, From b3c37fab5965366c5f2ee5b9eb63ec2929c37021 Mon Sep 17 00:00:00 2001 From: Rangel Stoilov Date: Tue, 31 Jan 2023 14:52:49 +0200 Subject: [PATCH 3/5] fix: expect proper error selector and require fail --- contracts/modules/Exec.sol | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/contracts/modules/Exec.sol b/contracts/modules/Exec.sol index bd468bde..5d6f6968 100644 --- a/contracts/modules/Exec.sol +++ b/contracts/modules/Exec.sol @@ -8,6 +8,7 @@ import "../PToken.sol"; import "../Interfaces.sol"; import "../Utils.sol"; + /// @notice Definition of callback method that deferLiquidityCheck will invoke on your contract interface IDeferredLiquidityCheck { function onDeferredLiquidityCheck(bytes memory data) external; @@ -120,25 +121,25 @@ contract Exec is BaseLogic { /// @param items List of operations to execute /// @param deferLiquidityChecks List of user accounts to defer liquidity checks for /// @dev During simulation all batch items are executed, regardless of the `allowError` flag - /// we revert the bytes if the length is 132, otherwise we decode the bytes to EulerBatchItemResponse[] - /// length of 132 is equal to the length of the revert message "e/batch/simulation-did-not-revert" function batchDispatchSimulateDecoded(EulerBatchItem[] calldata items, address[] calldata deferLiquidityChecks) external reentrantOK returns (EulerBatchItemResponse[] memory simulation) { address msgSender = unpackTrailingParamMsgSender(); bytes memory data = abi.encodeWithSelector(Exec.batchDispatchSimulate.selector, items, deferLiquidityChecks); bytes memory inputWrapped = abi.encodePacked(data, uint160(msgSender), uint160(msg.sender)); // msg.sender is the proxy address (bool success, bytes memory reason) = moduleLookup[MODULEID__EXEC].delegatecall(inputWrapped); - if (!success) { - if(reason.length == 132) { - revertBytes(reason); - } - assembly { - reason := add(reason, 0x4) - } - simulation = abi.decode( - reason, - (EulerBatchItemResponse[]) - ); + + require(!success, "e/batch/simulation-did-not-revert"); + + bytes4 errorSelector = bytes4(reason); + if(errorSelector != BatchDispatchSimulation.selector){ + revertBytes(reason); + } + assembly { + reason := add(reason, 0x4) } + simulation = abi.decode( + reason, + (EulerBatchItemResponse[]) + ); } From b456b59df461038e406e9bd83b6e09698823a9e0 Mon Sep 17 00:00:00 2001 From: Rangel Stoilov Date: Tue, 31 Jan 2023 18:14:30 +0200 Subject: [PATCH 4/5] test: malicious token test for simulation --- test/batch.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/batch.js b/test/batch.js index c371f645..16cca55e 100644 --- a/test/batch.js +++ b/test/batch.js @@ -232,6 +232,26 @@ et.testSet({ ] }) +.test({ + desc: "new batch simulate", + actions: ctx => [ + { send: 'eTokens.eTST.deposit', args: [0, et.eth(1)], }, + { send: 'tokens.TST.configure', args: ['transfer/revert', []] }, + { action: 'sendBatch', simulate: true, deferLiquidityChecks: [ctx.wallet.address], batch: [ + { send: 'eTokens.eTST.withdraw', args: [0, et.eth(1)], }, + ], onResult: r => { + et.expect(r[0].success).to.equal(false); + // safeTransferFrom adds additional error bytes that we need to remove + // first 4 bytes are the error selector + // second 32 bytes are the pointer for the start of error msg + // third 32 bytes are the length of the error msg + // then we have 4 bytes of again the Error(string) selector = 08c379a0 + // size = 4 + 32 + 32 + 4 = 72 which equals 144 length slice + 2 for 0x = 146 + const msg = utils.defaultAbiCoder.decode(["string"], "0x" + r[0].result.slice(146))[0]; + et.expect(msg).to.equal("revert behaviour") + }}, + ] +}) .run(); From d7d2117e2624a649038e4e425d6525af601fe76d Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Mon, 6 Mar 2023 19:37:49 +0100 Subject: [PATCH 5/5] fix: ECE-01S and ECE-01C --- contracts/modules/Exec.sol | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/contracts/modules/Exec.sol b/contracts/modules/Exec.sol index 5d6f6968..679ab37d 100644 --- a/contracts/modules/Exec.sol +++ b/contracts/modules/Exec.sol @@ -125,9 +125,7 @@ contract Exec is BaseLogic { address msgSender = unpackTrailingParamMsgSender(); bytes memory data = abi.encodeWithSelector(Exec.batchDispatchSimulate.selector, items, deferLiquidityChecks); bytes memory inputWrapped = abi.encodePacked(data, uint160(msgSender), uint160(msg.sender)); // msg.sender is the proxy address - (bool success, bytes memory reason) = moduleLookup[MODULEID__EXEC].delegatecall(inputWrapped); - - require(!success, "e/batch/simulation-did-not-revert"); + (, bytes memory reason) = moduleLookup[MODULEID__EXEC].delegatecall(inputWrapped); bytes4 errorSelector = bytes4(reason); if(errorSelector != BatchDispatchSimulation.selector){