check msg.data length (#13352)

* check msg.data length * Update gethwrappers --------- Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
smartcontractkit · May 31, 2024 · 33a9cdf · 33a9cdf
1 parent 46a1b9d
commit 33a9cdf
Show file tree

Hide file tree

Showing 8 changed files with 57 additions and 2 deletions.
diff --git a/.changeset/breezy-pears-cough.md b/.changeset/breezy-pears-cough.md
@@ -0,0 +1,5 @@
+---
+"chainlink": patch
+---
+
+check data size #bugfix
diff --git a/contracts/.changeset/smart-trainers-begin.md b/contracts/.changeset/smart-trainers-begin.md
@@ -0,0 +1,5 @@
+---
+"@chainlink/contracts": patch
+---
+
+check data size #bugfix
diff --git a/contracts/scripts/native_solc_compile_all_automation b/contracts/scripts/native_solc_compile_all_automation
@@ -103,5 +103,6 @@ compileContract automation/dev/v2_3/AutomationRegistrar2_3.sol
 compileContract automation/dev/v2_3/AutomationRegistry2_3.sol
 compileContract automation/dev/v2_3/AutomationRegistryLogicA2_3.sol
 compileContract automation/dev/v2_3/AutomationRegistryLogicB2_3.sol
+compileContract automation/dev/v2_3/AutomationRegistryLogicC2_3.sol
 compileContract automation/dev/v2_3/AutomationUtils2_3.sol
 compileContract automation/dev/interfaces/v2_3/IAutomationRegistryMaster2_3.sol
diff --git a/contracts/src/v0.8/automation/dev/test/AutomationRegistry2_3.t.sol b/contracts/src/v0.8/automation/dev/test/AutomationRegistry2_3.t.sol
@@ -1611,6 +1611,43 @@ contract BillingOverrides is SetUp {
 }
 
 contract Transmit is SetUp {
+  function test_transmitRevertWithExtraBytes() external {
+    bytes32[3] memory exampleReportContext = [
+      bytes32(0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef),
+      bytes32(0xabcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890),
+      bytes32(0x7890abcdef1234567890abcdef1234567890abcdef1234567890abcdef123456)
+    ];
+
+    bytes memory exampleRawReport = hex"deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef";
+
+    bytes32[] memory exampleRs = new bytes32[](3);
+    exampleRs[0] = bytes32(0x1234561234561234561234561234561234561234561234561234561234561234);
+    exampleRs[1] = bytes32(0x1234561234561234561234561234561234561234561234561234561234561234);
+    exampleRs[2] = bytes32(0x7890789078907890789078907890789078907890789078907890789078907890);
+
+    bytes32[] memory exampleSs = new bytes32[](3);
+    exampleSs[0] = bytes32(0x1234561234561234561234561234561234561234561234561234561234561234);
+    exampleSs[1] = bytes32(0x1234561234561234561234561234561234561234561234561234561234561234);
+    exampleSs[2] = bytes32(0x1234561234561234561234561234561234561234561234561234561234561234);
+
+    bytes32 exampleRawVs = bytes32(0x1234561234561234561234561234561234561234561234561234561234561234);
+
+    bytes memory transmitData = abi.encodeWithSelector(
+      registry.transmit.selector,
+      exampleReportContext,
+      exampleRawReport,
+      exampleRs,
+      exampleSs,
+      exampleRawVs
+    );
+    bytes memory badTransmitData = bytes.concat(transmitData, bytes1(0x00)); // add extra data
+    vm.startPrank(TRANSMITTERS[0]);
+    (bool success, bytes memory returnData) = address(registry).call(badTransmitData); // send the bogus transmit
+    assertFalse(success, "Call did not revert as expected");
+    assertEq(returnData, abi.encodePacked(Registry.InvalidDataLength.selector));
+    vm.stopPrank();
+  }
+
   function test_handlesMixedBatchOfBillingTokens() external {
     uint256[] memory prevUpkeepBalances = new uint256[](3);
     prevUpkeepBalances[0] = registry.getBalance(linkUpkeepID);

diff --git a/contracts/src/v0.8/automation/dev/v2_3/AutomationRegistry2_3.sol b/contracts/src/v0.8/automation/dev/v2_3/AutomationRegistry2_3.sol
@@ -90,6 +90,11 @@ contract AutomationRegistry2_3 is AutomationRegistryBase2_3, OCR2Abstract, Chain
     bytes32 rawVs
   ) external override {
     uint256 gasOverhead = gasleft();
+    // use this msg.data length check to ensure no extra data is included in the call
+    // 4 is first 4 bytes of the keccak-256 hash of the function signature. ss.length == rs.length so use one of them
+    // 4 + (32 * 3) + (rawReport.length + 32 + 32) + (32 * rs.length + 32 + 32) + (32 * ss.length + 32 + 32) + 32
+    uint256 requiredLength = 324 + rawReport.length + 64 * rs.length;
+    if (msg.data.length != requiredLength) revert InvalidDataLength();
     HotVars memory hotVars = s_hotVars;
 
     if (hotVars.paused) revert RegistryPaused();

diff --git a/...gethwrappers/generated/automation_registry_wrapper_2_3/automation_registry_wrapper_2_3.go b/...gethwrappers/generated/automation_registry_wrapper_2_3/automation_registry_wrapper_2_3.go
diff --git a/core/gethwrappers/generation/generated-wrapper-dependency-versions-do-not-edit.txt b/core/gethwrappers/generation/generated-wrapper-dependency-versions-do-not-edit.txt
@@ -13,8 +13,9 @@ automation_registry_logic_a_wrapper_2_2: ../../contracts/solc/v0.8.19/Automation
 automation_registry_logic_a_wrapper_2_3: ../../contracts/solc/v0.8.19/AutomationRegistryLogicA2_3/AutomationRegistryLogicA2_3.abi ../../contracts/solc/v0.8.19/AutomationRegistryLogicA2_3/AutomationRegistryLogicA2_3.bin 73b5cc3ece642abbf6f2a4c9188335b71404f4dd0ad10b761390b6397af6f1c8
 automation_registry_logic_b_wrapper_2_2: ../../contracts/solc/v0.8.19/AutomationRegistryLogicB2_2/AutomationRegistryLogicB2_2.abi ../../contracts/solc/v0.8.19/AutomationRegistryLogicB2_2/AutomationRegistryLogicB2_2.bin a6d33dfbbfb0ff253eb59a51f4f6d6d4c22ea5ec95aae52d25d49a312b37a22f
 automation_registry_logic_b_wrapper_2_3: ../../contracts/solc/v0.8.19/AutomationRegistryLogicB2_3/AutomationRegistryLogicB2_3.abi ../../contracts/solc/v0.8.19/AutomationRegistryLogicB2_3/AutomationRegistryLogicB2_3.bin fbf6f6cf4e6858855ff5da847c3baa4859dd997cfae51f2fa0651e4fa15b92c9
+automation_registry_logic_c_wrapper_2_3: ../../contracts/solc/v0.8.19/AutomationRegistryLogicC2_3/AutomationRegistryLogicC2_3.abi ../../contracts/solc/v0.8.19/AutomationRegistryLogicC2_3/AutomationRegistryLogicC2_3.bin 3ee51aa2f946b9fe3583b4a8526d29721339f96774e410bd37ddfe8184a63701
 automation_registry_wrapper_2_2: ../../contracts/solc/v0.8.19/AutomationRegistry2_2/AutomationRegistry2_2.abi ../../contracts/solc/v0.8.19/AutomationRegistry2_2/AutomationRegistry2_2.bin de60f69878e9b32a291a001c91fc8636544c2cfbd9b507c8c1a4873b602bfb62
-automation_registry_wrapper_2_3: ../../contracts/solc/v0.8.19/AutomationRegistry2_3/AutomationRegistry2_3.abi ../../contracts/solc/v0.8.19/AutomationRegistry2_3/AutomationRegistry2_3.bin 10078161924b38cf968ceb65f54078412832ada9abeebcd011ee7291811921c2
+automation_registry_wrapper_2_3: ../../contracts/solc/v0.8.19/AutomationRegistry2_3/AutomationRegistry2_3.abi ../../contracts/solc/v0.8.19/AutomationRegistry2_3/AutomationRegistry2_3.bin f8f920a225fdb1e36948dd95bae3aa46ecc2b01fd113480e111960b5e5f95624
 automation_utils_2_1: ../../contracts/solc/v0.8.16/AutomationUtils2_1/AutomationUtils2_1.abi ../../contracts/solc/v0.8.16/AutomationUtils2_1/AutomationUtils2_1.bin 815b17b63f15d26a0274b962eefad98cdee4ec897ead58688bbb8e2470e585f5
 automation_utils_2_2: ../../contracts/solc/v0.8.19/AutomationUtils2_2/AutomationUtils2_2.abi ../../contracts/solc/v0.8.19/AutomationUtils2_2/AutomationUtils2_2.bin 8743f6231aaefa3f2a0b2d484258070d506e2d0860690e66890dccc3949edb2e
 automation_utils_2_3: ../../contracts/solc/v0.8.19/AutomationUtils2_3/AutomationUtils2_3.abi ../../contracts/solc/v0.8.19/AutomationUtils2_3/AutomationUtils2_3.bin 11e2b481dc9a4d936e3443345d45d2cc571164459d214917b42a8054b295393b

diff --git a/core/gethwrappers/go_generate.go b/core/gethwrappers/go_generate.go
@@ -48,6 +48,7 @@ package gethwrappers
 //go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.19/AutomationRegistry2_3/AutomationRegistry2_3.abi ../../contracts/solc/v0.8.19/AutomationRegistry2_3/AutomationRegistry2_3.bin AutomationRegistry automation_registry_wrapper_2_3
 //go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.19/AutomationRegistryLogicA2_3/AutomationRegistryLogicA2_3.abi ../../contracts/solc/v0.8.19/AutomationRegistryLogicA2_3/AutomationRegistryLogicA2_3.bin AutomationRegistryLogicA automation_registry_logic_a_wrapper_2_3
 //go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.19/AutomationRegistryLogicB2_3/AutomationRegistryLogicB2_3.abi ../../contracts/solc/v0.8.19/AutomationRegistryLogicB2_3/AutomationRegistryLogicB2_3.bin AutomationRegistryLogicB automation_registry_logic_b_wrapper_2_3
+//go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.19/AutomationRegistryLogicC2_3/AutomationRegistryLogicC2_3.abi ../../contracts/solc/v0.8.19/AutomationRegistryLogicC2_3/AutomationRegistryLogicC2_3.bin AutomationRegistryLogicC automation_registry_logic_c_wrapper_2_3
 //go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.19/IAutomationRegistryMaster2_3/IAutomationRegistryMaster2_3.abi ../../contracts/solc/v0.8.19/IAutomationRegistryMaster2_3/IAutomationRegistryMaster2_3.bin IAutomationRegistryMaster2_3 i_automation_registry_master_wrapper_2_3
 //go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.19/AutomationUtils2_3/AutomationUtils2_3.abi ../../contracts/solc/v0.8.19/AutomationUtils2_3/AutomationUtils2_3.bin AutomationUtils automation_utils_2_3
 //go:generate go run ./generation/generate/wrap.go ../../contracts/solc/v0.8.19/ArbitrumModule/ArbitrumModule.abi ../../contracts/solc/v0.8.19/ArbitrumModule/ArbitrumModule.bin ArbitrumModule arbitrum_module