Skip to content

Commit 732d64f

Browse files
committed
Source/common/verifyinghasher: address open CodeRabbit nits
* HeaderParserTest: switch testParsesFatBinary, testChunkSizeInvariance, and testSliceOffsetIfKnown from /usr/bin/file to the checked-in hw_universal fixture for determinism. The system binary is fat today, but Apple has been progressively shipping arm64-only system binaries; a future CI runner could silently lose fat coverage in these tests. The fixture carries arm64 + x86_64 (no arm64e), so a host-conditional kHwUniversalArch selector picks the slice actually present rather than reusing kHostArch (which resolves to arm64e on Apple Silicon). * HeaderParserTest: rework testRejectsImplausibleCsBlobSize to feed just the header + LC prefix (~48 bytes) instead of allocating a 256 MiB zero-filled buffer per test run. The parser hits the kMaxCsBlobSize cap on the LC_CODE_SIGNATURE entry before reading the declared CS region, so we tell it the logical file size via kTotal and only hand it the prefix.
1 parent 8762323 commit 732d64f

2 files changed

Lines changed: 39 additions & 14 deletions

File tree

Source/common/verifyinghasher/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ objc_library(
7979
santa_unit_test(
8080
name = "HeaderParserTest",
8181
srcs = ["HeaderParserTest.mm"],
82+
structured_resources = [":hw_universal_fixture"],
8283
deps = [
8384
":FileReader",
8485
":HeaderParser",

Source/common/verifyinghasher/HeaderParserTest.mm

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,15 @@
4545
constexpr ArchSelector kHostArch = {CPU_TYPE_X86_64, CPU_SUBTYPE_X86_64_ALL};
4646
#endif
4747

48+
// hw_universal carries arm64 + x86_64 slices (no arm64e). Fat tests
49+
// that drive the parser with the checked-in fixture must pick the
50+
// slice actually present for the running host, not kHostArch.
51+
#if defined(__arm64__) || defined(__aarch64__)
52+
constexpr ArchSelector kHwUniversalArch = {CPU_TYPE_ARM64, CPU_SUBTYPE_ARM64_ALL};
53+
#else
54+
constexpr ArchSelector kHwUniversalArch = {CPU_TYPE_X86_64, CPU_SUBTYPE_X86_64_ALL};
55+
#endif
56+
4857
// Read entire file into a vector for synthetic feed.
4958
std::vector<uint8_t> Slurp(const char* path) {
5059
santa::ScopedFile sf(::open(path, O_RDONLY | O_CLOEXEC));
@@ -113,6 +122,11 @@ @interface HeaderParserTest : XCTestCase
113122

114123
@implementation HeaderParserTest
115124

125+
- (NSString*)hwUniversalFixturePath {
126+
NSBundle* bundle = [NSBundle bundleForClass:[self class]];
127+
return [bundle.resourcePath stringByAppendingPathComponent:@"testdata/hw_universal"];
128+
}
129+
116130
- (void)testParsesThinBinary {
117131
auto data = Slurp("/usr/bin/yes");
118132
XCTAssertGreaterThanOrEqual(data.size(), 4u);
@@ -125,22 +139,25 @@ - (void)testParsesThinBinary {
125139
}
126140

127141
- (void)testParsesFatBinary {
128-
auto data = Slurp("/usr/bin/file");
142+
auto data = Slurp([self hwUniversalFixturePath].UTF8String);
129143
santa::SliceInfo slice{};
130144
std::string err;
131-
auto s = FeedChunked(data, kHostArch, 1u << 20, &slice, &err);
145+
auto s = FeedChunked(data, kHwUniversalArch, 1u << 20, &slice, &err);
132146
XCTAssertEqual(s, HeaderParser::Status::kReady, @"FeedChunked: %s", err.c_str());
133147
XCTAssertGreaterThan(slice.slice_offset, 0u); // fat: slice not at offset 0
134148
XCTAssertGreaterThan(slice.cs_blob_size, 0u);
135149
}
136150

137151
- (void)testChunkSizeInvariance {
138-
auto data = Slurp("/usr/bin/file");
152+
auto data = Slurp([self hwUniversalFixturePath].UTF8String);
139153
santa::SliceInfo big{}, small{}, byteByByte{};
140154
std::string err;
141-
XCTAssertEqual(FeedChunked(data, kHostArch, 1u << 20, &big, &err), HeaderParser::Status::kReady);
142-
XCTAssertEqual(FeedChunked(data, kHostArch, 17, &small, &err), HeaderParser::Status::kReady);
143-
XCTAssertEqual(FeedChunked(data, kHostArch, 1, &byteByByte, &err), HeaderParser::Status::kReady);
155+
XCTAssertEqual(FeedChunked(data, kHwUniversalArch, 1u << 20, &big, &err),
156+
HeaderParser::Status::kReady);
157+
XCTAssertEqual(FeedChunked(data, kHwUniversalArch, 17, &small, &err),
158+
HeaderParser::Status::kReady);
159+
XCTAssertEqual(FeedChunked(data, kHwUniversalArch, 1, &byteByByte, &err),
160+
HeaderParser::Status::kReady);
144161
XCTAssertEqual(big.cs_blob_offset, small.cs_blob_offset);
145162
XCTAssertEqual(big.cs_blob_offset, byteByByte.cs_blob_offset);
146163
XCTAssertEqual(big.cs_blob_size, small.cs_blob_size);
@@ -240,23 +257,30 @@ - (void)testRejectsDataoffPastSlice {
240257
// rejected by the size cap before it can drive a giant cs_blob_buf_ alloc.
241258
// We need a slice large enough that the size check trips, not the slice
242259
// bounds — pick datasize > kMaxCsBlobSize (64 MiB) and slice_size larger.
260+
//
261+
// The parser rejects on the LC_CODE_SIGNATURE entry before reading the
262+
// declared CS region, so we feed only the header + LC prefix and tell
263+
// the parser the logical file size separately via kTotal. Avoids
264+
// allocating a 256 MiB zero-filled buffer per test run.
243265
- (void)testRejectsImplausibleCsBlobSize {
244-
constexpr uint64_t kTotal = 256ull * 1024 * 1024; // 256 MiB
266+
constexpr uint64_t kTotal = 256ull * 1024 * 1024; // logical file size
267+
constexpr size_t kPrefix = sizeof(struct mach_header_64) + sizeof(struct linkedit_data_command);
245268
// dataoff = 0x1000, datasize = 128 MiB → exceeds the 64 MiB cap.
246-
auto data = MakeThin64WithCsBlob(0x1000, 128u * 1024 * 1024, kTotal);
247-
ArchSelector want{CPU_TYPE_X86_64, CPU_SUBTYPE_X86_64_ALL};
248-
std::string err;
249-
auto s = FeedChunked(data, want, /*chunk=*/65536, nullptr, &err);
269+
auto data = MakeThin64WithCsBlob(/*cs_dataoff=*/0x1000,
270+
/*cs_datasize=*/128u * 1024 * 1024,
271+
/*total_size=*/kPrefix);
272+
HeaderParser p({CPU_TYPE_X86_64, CPU_SUBTYPE_X86_64_ALL}, kTotal);
273+
auto s = p.Update(data.data(), data.size(), /*chunk_off=*/0);
250274
XCTAssertEqual(s, HeaderParser::Status::kError);
251-
XCTAssertTrue(err.find("implausibly large") != std::string::npos);
275+
XCTAssertTrue(std::string(p.LastError()).find("implausibly large") != std::string::npos);
252276
}
253277

254278
// H1 regression: SliceOffsetIfKnown must report nullopt until the parser has
255279
// resolved the slice (post-magic for thin, post-fat-arch-table for fat) and
256280
// then return the same offset that ends up on Slice() at kReady.
257281
- (void)testSliceOffsetIfKnown {
258-
auto data = Slurp("/usr/bin/file"); // fat binary
259-
HeaderParser p(kHostArch, data.size());
282+
auto data = Slurp([self hwUniversalFixturePath].UTF8String); // fat binary
283+
HeaderParser p(kHwUniversalArch, data.size());
260284
// Before any bytes: definitely unknown.
261285
XCTAssertFalse(p.SliceOffsetIfKnown().has_value());
262286

0 commit comments

Comments
 (0)