From fac0b6893477da1c69a5fd70b4a18ef4bd8a4a0f Mon Sep 17 00:00:00 2001
From: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Fri, 31 Jul 2026 15:51:20 -0700
Subject: [PATCH] [Crx] Disallow EOCD64 Record tokens in crx file headers

Just as we disallow EOCD and EOCD64 tokens, we should also disallow
EOCD64 record tokens.

Guard against these, behind a default-enabled feature flag (out of an
abundance of caution), and add a regression test.

Bug: 538715523
Change-Id: Ic5a5ac5e93412e42d3d0c32cb1e07cdcf20f552b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8176148
Commit-Queue: Devlin Cronin <rdevlin.cronin@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1672146}
---

diff --git a/components/crx_file/crx_verifier.cc b/components/crx_file/crx_verifier.cc
index 77a54cf..2b91e49 100644
--- a/components/crx_file/crx_verifier.cc
+++ b/components/crx_file/crx_verifier.cc
@@ -5,6 +5,7 @@
 #include "components/crx_file/crx_verifier.h"
 
 #include <algorithm>
+#include <array>
 #include <climits>
 #include <cstring>
 #include <iterator>
@@ -14,6 +15,8 @@
 #include <utility>
 
 #include "base/base64.h"
+#include "base/feature.h"
+#include "base/feature_list.h"
 #include "base/files/file.h"
 #include "base/files/file_path.h"
 #include "base/functional/bind.h"
@@ -35,6 +38,11 @@
 
 using KeyHash = std::array<uint8_t, crypto::hash::kSha256Size>;
 
+// A feature to block EOCD64 record tokens in CRX files, only here as a
+// killswitch. This can be removed in October 2026.
+BASE_FEATURE(kDisallowEocdRecord64TokensInCrx,
+             base::FEATURE_ENABLED_BY_DEFAULT);
+
 // The SHA256 hash of the DER SPKI "ecdsa_2017_public" Crx3 key.
 constexpr KeyHash kPublisherKeyHash = {
     0x61, 0xf7, 0xf2, 0xa6, 0xbf, 0xcf, 0x74, 0xcd, 0x0b, 0xc1, 0xfe,
@@ -49,6 +57,7 @@
 
 constexpr auto kEocd = std::to_array<uint8_t>({'P', 'K', 0x05, 0x06});
 constexpr auto kEocd64 = std::to_array<uint8_t>({'P', 'K', 0x06, 0x07});
+constexpr auto kEocd64Record = std::to_array<uint8_t>({'P', 'K', 0x06, 0x06});
 
 using VerifierCollection = std::vector<std::unique_ptr<crypto::sign::Verifier>>;
 using RepeatedProof = ::google::protobuf::RepeatedPtrField<AsymmetricKeyProof>;
@@ -122,12 +131,19 @@
     return VerifierResult::ERROR_HEADER_INVALID;
   }
 
-  // If the header contains a ZIP EOCD or EOCD64 token, unzipping may not work
-  // correctly.
+  // If the header contains a ZIP EOCD, EOCD64, or EOCD64 record token,
+  // unzipping may not work correctly.
   if (std::ranges::search(header_bytes, kEocd) ||
       std::ranges::search(header_bytes, kEocd64)) {
     return VerifierResult::ERROR_HEADER_INVALID;
   }
+  // Out of an abundance of caution, we gate the EOCD64 record token on a
+  // base::Feature. The feature check can be removed (and this can be folded
+  // into the if-statement above) in October 2026.
+  if (base::FeatureList::IsEnabled(kDisallowEocdRecord64TokensInCrx) &&
+      std::ranges::search(header_bytes, kEocd64Record)) {
+    return VerifierResult::ERROR_HEADER_INVALID;
+  }
 
   CrxFileHeader header;
   if (!header.ParseFromArray(header_bytes.data(), header_size)) {
diff --git a/components/crx_file/crx_verifier_unittest.cc b/components/crx_file/crx_verifier_unittest.cc
index e47f963..8b5bba2 100644
--- a/components/crx_file/crx_verifier_unittest.cc
+++ b/components/crx_file/crx_verifier_unittest.cc
@@ -3,8 +3,14 @@
 // found in the LICENSE file.
 
 #include "components/crx_file/crx_verifier.h"
+
+#include <string>
+#include <vector>
+
 #include "base/base_paths.h"
 #include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
 #include "base/path_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -262,4 +268,39 @@
   EXPECT_TRUE(compressed_verified_contents.empty());
 }
 
+// Tests that we properly reject a crx file that includes an EOCD64 Record
+// token. Regression test for https://crbug.com/538715523.
+TEST_F(CrxVerifierTest, RejectsEocd64RecordInHeader) {
+  const std::vector<std::vector<uint8_t>> keys;
+  const std::vector<uint8_t> hash;
+  std::string public_key = "UNSET";
+  std::string crx_id = "UNSET";
+
+  std::string crx_contents;
+  ASSERT_TRUE(base::ReadFileToString(TestFile("valid_no_publisher.crx3"),
+                                     &crx_contents));
+
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+  // Insert PK\x06\x06 (Zip64 EOCD Record) into header_bytes (starts at offset
+  // 12).
+  std::string crx_with_eocd64_record = crx_contents;
+  ASSERT_GT(crx_with_eocd64_record.size(), 20u);
+  crx_with_eocd64_record[12] = 'P';
+  crx_with_eocd64_record[13] = 'K';
+  crx_with_eocd64_record[14] = 0x06;
+  crx_with_eocd64_record[15] = 0x06;
+
+  base::FilePath test_file =
+      temp_dir.GetPath().AppendASCII("eocd64record.crx3");
+  ASSERT_TRUE(base::WriteFile(test_file, crx_with_eocd64_record));
+
+  EXPECT_EQ(VerifierResult::ERROR_HEADER_INVALID,
+            Verify(test_file, VerifierFormat::CRX3, keys, hash, &public_key,
+                   &crx_id, /*compressed_verified_contents=*/nullptr));
+  EXPECT_EQ("UNSET", crx_id);
+  EXPECT_EQ("UNSET", public_key);
+}
+
 }  // namespace crx_file

diff --git a/components/crx_file/crx_verifier_unittest.cc b/components/crx_file/crx_verifier_unittest.cc
index e47f963..8b5bba2 100644
--- a/components/crx_file/crx_verifier_unittest.cc
+++ b/components/crx_file/crx_verifier_unittest.cc
@@ -3,8 +3,14 @@
 // found in the LICENSE file.
 
 #include "components/crx_file/crx_verifier.h"
+
+#include <string>
+#include <vector>
+
 #include "base/base_paths.h"
 #include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
 #include "base/path_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -262,4 +268,39 @@
   EXPECT_TRUE(compressed_verified_contents.empty());
 }
 
+// Tests that we properly reject a crx file that includes an EOCD64 Record
+// token. Regression test for https://crbug.com/538715523.
+TEST_F(CrxVerifierTest, RejectsEocd64RecordInHeader) {
+  const std::vector<std::vector<uint8_t>> keys;
+  const std::vector<uint8_t> hash;
+  std::string public_key = "UNSET";
+  std::string crx_id = "UNSET";
+
+  std::string crx_contents;
+  ASSERT_TRUE(base::ReadFileToString(TestFile("valid_no_publisher.crx3"),
+                                     &crx_contents));
+
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+  // Insert PK\x06\x06 (Zip64 EOCD Record) into header_bytes (starts at offset
+  // 12).
+  std::string crx_with_eocd64_record = crx_contents;
+  ASSERT_GT(crx_with_eocd64_record.size(), 20u);
+  crx_with_eocd64_record[12] = 'P';
+  crx_with_eocd64_record[13] = 'K';
+  crx_with_eocd64_record[14] = 0x06;
+  crx_with_eocd64_record[15] = 0x06;
+
+  base::FilePath test_file =
+      temp_dir.GetPath().AppendASCII("eocd64record.crx3");
+  ASSERT_TRUE(base::WriteFile(test_file, crx_with_eocd64_record));
+
+  EXPECT_EQ(VerifierResult::ERROR_HEADER_INVALID,
+            Verify(test_file, VerifierFormat::CRX3, keys, hash, &public_key,
+                   &crx_id, /*compressed_verified_contents=*/nullptr));
+  EXPECT_EQ("UNSET", crx_id);
+  EXPECT_EQ("UNSET", public_key);
+}
+
 }  // namespace crx_file