Source/common/verifyinghasher: address review feedback

Bundles review-driven changes:

- VerifyingHasherCore.mm phase 5 (tail) now returns kIoError when pread
  yields 0 bytes with cursor_ < total. The previous silent break
  finalized the SHA-256 over only a prefix of the file and returned
  kOk / kPagesMismatched, violating the "32-byte SHA-256 of the full
  file" contract. Matches phases 1 and 3's existing premature-EOF
  handling. VerifyingHasherCoreTest.mm adds a regression test using
  a local TruncatedMemoryFileReader that claims a Size() larger than
  its data to simulate mid-verification truncation.

- MemoryFileReader and CountingMemoryFileReader now set errno before
  returning -1 (EINVAL on negative offset, EIO on
  MemoryFileReader's synthetic ScheduleErrorOnNextPread() path).
  FileReader.h's interface documents "-1 = error (errno set)";
  FdFileReader satisfied this via pread() itself, but the in-memory
  readers left errno untouched.

- CodeSignatureParserTest.mm's ExtractCsBlob fast-fails when the fat
  scan finds no matching architecture, rather than falling through
  into mach_header_64 parsing of the fat header bytes (which could
  worst-case allocate std::vector<uint8_t> for an arbitrary uint32).

- install_libclang_fuzzer.sh derives the resource directory via
  `clang -print-resource-dir` instead of parsing the human-facing
  `clang --version` output. Also removes a latent dependency on the
  /clang/<full-version> -> /clang/<major> compatibility symlink, which
  isn't created on all Xcode installs.

- HashTraits.h's VERIFYINGHASHER_CC_UPDATE_LOOP macro renames its
  loop-locals from _p/_n/_step_sz/_step/_rc to vh_-prefixed names.
  All current expansion sites are at block scope where the underscore
  names are permitted, but at namespace scope they're reserved; the
  prefix also avoids silent shadowing of caller-side identifiers.

- CodeSignatureParser.h drops an unused <vector> include. The header's
  ParsedCodeDirectory and ParseCodeSignature declarations use std::span
  / std::string only.

Author: mlw

Source/common/verifyinghasher/CodeSignatureParser.h

@@ -26,7 +26,6 @@ __END_DECLS
 #include <span>
 #include <string>
 #include <string_view>
-#include <vector>
 
 namespace santa {
 

Source/common/verifyinghasher/CodeSignatureParserTest.mm

@@ -69,14 +69,23 @@
 #else
     cpu_type_t want = CPU_TYPE_X86_64;
 #endif
+    bool matched = false;
     for (auto& a : archs) {
       cpu_type_t ct = static_cast<cpu_type_t>(OSSwapBigToHostInt32(a.cputype));
       if (ct == want) {
         slice_off = OSSwapBigToHostInt32(a.offset);
         slice_size = OSSwapBigToHostInt32(a.size);
+        matched = true;
         break;
       }
     }
+    // Fast-fail: without this, slice_off stays 0 and the load-command
+    // walk below reads the fat header bytes as a mach_header_64 and
+    // allocates std::vector<uint8_t> for an arbitrary mh.sizeofcmds.
+    if (!matched) {
+      ::close(fd);
+      return {};
+    }
   }
   // Walk the slice's load commands to find LC_CODE_SIGNATURE.
   struct mach_header_64 mh{};

Source/common/verifyinghasher/CountingMemoryFileReader.h

@@ -16,6 +16,7 @@
 #define SANTA_COMMON_VERIFYINGHASHER_COUNTINGMEMORYFILEREADER_H
 
 #include <algorithm>
+#include <cerrno>
 #include <cstdint>
 #include <cstring>
 #include <vector>
@@ -34,7 +35,10 @@ class CountingMemoryFileReader : public FileReader {
   explicit CountingMemoryFileReader(std::vector<uint8_t> data)
       : data_(std::move(data)), reads_(data_.size(), 0) {}
   ssize_t Pread(void* buf, size_t len, off_t off) override {
-    if (off < 0) return -1;
+    if (off < 0) {
+      errno = EINVAL;
+      return -1;
+    }
     if (static_cast<size_t>(off) >= data_.size()) return 0;
     size_t n = std::min(len, data_.size() - static_cast<size_t>(off));
     std::memcpy(buf, data_.data() + off, n);

Source/common/verifyinghasher/HashTraits.h

@@ -48,19 +48,19 @@ namespace detail {
 // silently truncate, corrupting the digest. Wrap the underlying CC call in
 // a loop that splits inputs into UINT32_MAX-sized chunks. Returns 1 on
 // success, 0 on the first underlying failure (matching CC convention).
-#define VERIFYINGHASHER_CC_UPDATE_LOOP(cc_update_fn, ctx, data, len) \
-  do {                                                               \
-    const auto* _p = static_cast<const uint8_t*>(data);              \
-    size_t _n = (len);                                               \
-    while (_n > 0) {                                                 \
-      const size_t _step_sz = (_n > UINT32_MAX) ? UINT32_MAX : _n;   \
-      const CC_LONG _step = static_cast<CC_LONG>(_step_sz);          \
-      const int _rc = cc_update_fn((ctx), _p, _step);                \
-      if (_rc != 1) return _rc;                                      \
-      _p += _step;                                                   \
-      _n -= _step_sz;                                                \
-    }                                                                \
-    return 1;                                                        \
+#define VERIFYINGHASHER_CC_UPDATE_LOOP(cc_update_fn, ctx, data, len)        \
+  do {                                                                      \
+    const auto* vh_p = static_cast<const uint8_t*>(data);                   \
+    size_t vh_n = (len);                                                    \
+    while (vh_n > 0) {                                                      \
+      const size_t vh_step_sz = (vh_n > UINT32_MAX) ? UINT32_MAX : vh_n;    \
+      const CC_LONG vh_step = static_cast<CC_LONG>(vh_step_sz);             \
+      const int vh_rc = cc_update_fn((ctx), vh_p, vh_step);                 \
+      if (vh_rc != 1) return vh_rc;                                         \
+      vh_p += vh_step;                                                      \
+      vh_n -= vh_step_sz;                                                   \
+    }                                                                       \
+    return 1;                                                               \
   } while (0)
 
 struct Sha256Algo {

Source/common/verifyinghasher/MemoryFileReader.h

@@ -16,6 +16,7 @@
 #define SANTA_COMMON_VERIFYINGHASHER_MEMORYFILEREADER_H
 
 #include <algorithm>
+#include <cerrno>
 #include <cstdint>
 #include <cstring>
 #include <vector>
@@ -33,9 +34,13 @@ class MemoryFileReader : public FileReader {
   ssize_t Pread(void* buf, size_t len, off_t off) override {
     if (fail_next_) {
       fail_next_ = false;
+      errno = EIO;
+      return -1;
+    }
+    if (off < 0) {
+      errno = EINVAL;
       return -1;
     }
-    if (off < 0) return -1;
     if (static_cast<size_t>(off) >= data_.size()) return 0;
     size_t available = data_.size() - static_cast<size_t>(off);
     size_t n = std::min(len, available);

Source/common/verifyinghasher/VerifyingHasherCore.mm

@@ -216,7 +216,15 @@
       last_error_ = "pread failed in tail phase";
       return Status::kIoError;
     }
-    if (n == 0) break;
+    // n == 0 with cursor_ < total means the underlying source returned
+    // EOF before we hit the size fstat reported at Run() entry (e.g.,
+    // the file was truncated mid-verification). Treat as kIoError —
+    // same as phase 3 — so the partial digest doesn't get finalized
+    // and returned as if it covered the full file.
+    if (n == 0) {
+      last_error_ = "unexpected EOF in tail phase";
+      return Status::kIoError;
+    }
     Sha256Traits::Update(&full_ctx_, chunk_buf_.data(), static_cast<size_t>(n));
     cursor_ += static_cast<uint64_t>(n);
   }

Source/common/verifyinghasher/VerifyingHasherCoreTest.mm

@@ -205,6 +205,32 @@
   return std::vector<uint8_t>(bytes.data() + sig_off, bytes.data() + sig_off + sig_size);
 }
 
+// Simulates a file truncated between fstat() and a later pread(): Size()
+// reports a larger value than the data actually contains, so a read past
+// data_.size() returns 0 (EOF) while the verifier still believes there
+// are more bytes to consume.
+class TruncatedMemoryFileReader : public santa::FileReader {
+ public:
+  TruncatedMemoryFileReader(std::vector<uint8_t> data, off_t claimed_size)
+      : data_(std::move(data)), claimed_size_(claimed_size) {}
+  ssize_t Pread(void* buf, size_t len, off_t off) override {
+    if (off < 0) {
+      errno = EINVAL;
+      return -1;
+    }
+    if (static_cast<size_t>(off) >= data_.size()) return 0;
+    size_t available = data_.size() - static_cast<size_t>(off);
+    size_t n = std::min(len, available);
+    std::memcpy(buf, data_.data() + off, n);
+    return static_cast<ssize_t>(n);
+  }
+  off_t Size() const override { return claimed_size_; }
+
+ private:
+  std::vector<uint8_t> data_;
+  off_t claimed_size_;
+};
+
 }  // namespace
 
 @interface VerifyingHasherCoreTest : XCTestCase
@@ -282,6 +308,26 @@ - (void)testIoErrorPropagated {
   XCTAssertTrue(v.FullFileDigest().empty());
 }
 
+// Phase 5 (tail) used to silently `break` when pread returned 0 with
+// cursor_ < total, finalizing the digest over only a prefix of the
+// file. The fix classifies that as kIoError, matching phases 1 and 3.
+// We simulate the trigger with a reader that claims a larger Size()
+// than its actual data — the verifier completes header / cs-blob /
+// signed-region phases normally, then the tail drain hits EOF early.
+- (void)testTailEofClassifiedAsIoError {
+  auto bytes = Slurp("/usr/bin/yes");
+  XCTAssertFalse(bytes.empty());
+  const off_t real_size = static_cast<off_t>(bytes.size());
+  TruncatedMemoryFileReader r(std::move(bytes), real_size + 1024 * 1024);
+  VerifyingHasherCore v(r, kHostArch);
+  auto s = v.Run();
+  XCTAssertEqual(s, VerifyingHasherCore::Status::kIoError);
+  XCTAssertTrue(v.FullFileDigest().empty());
+  XCTAssertNotEqual(std::string(v.LastError()).find("tail"), std::string::npos,
+                    @"LastError should point at the tail phase: %s",
+                    std::string(v.LastError()).c_str());
+}
+
 - (void)testSinglePassInvariant {
   auto bytes = Slurp("/usr/bin/yes");
   XCTAssertFalse(bytes.empty());

Testing/Fuzzing/install_libclang_fuzzer.sh

@@ -43,15 +43,17 @@ TARBALL_URL="https://github.com/llvm/llvm-project/releases/download/llvmorg-${LL
 # Major version only (e.g. "22" from "22.1.4") — matches the lib path inside the tarball.
 TARBALL_CLANG_MAJOR="${LLVM_VERSION%%.*}"
 
-# DST_PATH uses Apple Clang's version string (from `clang --version`) because
-# the Xcode toolchain directory layout follows Apple's numbering, not LLVM's.
-# Only the download URL needs the upstream LLVM version.
-CLANG_VERSION=$(clang --version | head -n 1 | cut -d' ' -f 4)
-if [[ -z "${CLANG_VERSION}" ]]; then
-  echo "ERROR: failed to parse Apple Clang version from 'clang --version'" >&2
+# Ask clang directly for its resource directory. On Apple toolchains this
+# resolves to .../Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/<apple-version>,
+# so we avoid parsing `clang --version` — whose human-facing format Apple has
+# changed in the past — and we avoid hand-concatenating the Xcode prefix.
+# Only the download URL still needs the upstream LLVM version (used above).
+RESOURCE_DIR=$(clang -print-resource-dir)
+if [[ -z "${RESOURCE_DIR}" ]]; then
+  echo "ERROR: 'clang -print-resource-dir' returned no output" >&2
   exit 1
 fi
-DST_PATH="$(xcode-select -p)/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/${CLANG_VERSION}/lib/darwin/libclang_rt.fuzzer_osx.a"
+DST_PATH="${RESOURCE_DIR}/lib/darwin/libclang_rt.fuzzer_osx.a"
 
 if [ -f "${DST_PATH}" ]; then
   echo "libclang_rt.fuzzer_osx.a already present at ${DST_PATH}, nothing to do."