From 941ee777f0b1914ad80fcda46cd711e09c1056c6 Mon Sep 17 00:00:00 2001 From: Dev Kalra Date: Wed, 13 Dec 2023 16:42:21 +0530 Subject: [PATCH] [entropy] audit - 2. lack of contract existence check (#1177) * contract existence check * better comment --- .../contracts/contracts/executor/Executor.sol | 10 +++++++- .../contracts/executor/ExecutorErrors.sol | 2 ++ .../contracts/forge-test/Executor.t.sol | 24 +++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/target_chains/ethereum/contracts/contracts/executor/Executor.sol b/target_chains/ethereum/contracts/contracts/executor/Executor.sol index 892cb93e..e2801d29 100644 --- a/target_chains/ethereum/contracts/contracts/executor/Executor.sol +++ b/target_chains/ethereum/contracts/contracts/executor/Executor.sol @@ -76,8 +76,16 @@ contract Executor { gi.executorAddress != address(this) ) revert ExecutorErrors.DeserializationError(); + // Check if the gi.callAddress is a contract account. + uint len; + address callAddress = address(gi.callAddress); + assembly { + len := extcodesize(callAddress) + } + if (len == 0) revert ExecutorErrors.InvalidContractTarget(); + bool success; - (success, response) = address(gi.callAddress).call(gi.callData); + (success, response) = address(callAddress).call(gi.callData); // Check if the call was successful or not. if (!success) { diff --git a/target_chains/ethereum/contracts/contracts/executor/ExecutorErrors.sol b/target_chains/ethereum/contracts/contracts/executor/ExecutorErrors.sol index 47cb905d..ae18743c 100644 --- a/target_chains/ethereum/contracts/contracts/executor/ExecutorErrors.sol +++ b/target_chains/ethereum/contracts/contracts/executor/ExecutorErrors.sol @@ -14,4 +14,6 @@ library ExecutorErrors { error DeserializationError(); // The message is not intended for this contract. error InvalidGovernanceTarget(); + // The target address for the contract call is not a contract + error InvalidContractTarget(); } diff --git a/target_chains/ethereum/contracts/forge-test/Executor.t.sol b/target_chains/ethereum/contracts/forge-test/Executor.t.sol index 1251b889..da2f40b6 100644 --- a/target_chains/ethereum/contracts/forge-test/Executor.t.sol +++ b/target_chains/ethereum/contracts/forge-test/Executor.t.sol @@ -344,6 +344,30 @@ contract ExecutorTest is Test, WormholeTestUtils { vm.expectRevert("call should revert"); executor.execute(vaa); } + + function testCallToEoaReverts() public { + bytes memory payload = abi.encodePacked( + uint32(0x5054474d), + PythGovernanceInstructions.GovernanceModule.EvmExecutor, + Executor.ExecutorAction.Execute, + CHAIN_ID, + address(executor), + address(100), + abi.encodeWithSelector(ICallable.foo.selector) + ); + + bytes memory vaa = generateVaa( + uint32(block.timestamp), + OWNER_CHAIN_ID, + OWNER_EMITTER, + 1, + payload, + NUM_SIGNERS + ); + + vm.expectRevert(ExecutorErrors.InvalidContractTarget.selector); + executor.execute(vaa); + } } interface ICallable {