fix(pack): a garbage e_lfanew threw instead of answering "not a PE"

`identify()` is documented as never throwing and, for one input, terminated
the process. `std::string_view::substr` throws `std::out_of_range` when
`pos > size()`, and the position came straight out of the file:

    if (b.substr(*lfanew, 4) == "PE\0\0")

A file that starts with "MZ" and has garbage at 0x3C is ordinary malformed
input — a truncated download, a DOS stub, a text file named `.exe` — and
`mcpp pack` calls this on every artifact it packages. Measured rather than
argued: `std::string_view{256 bytes}.substr(0xFFFFFFFF, 4)` throws
`string_view::substr` under libc++.

Every other read in the module goes through a bounds-checked accessor; these
two comparisons were the exceptions, which is exactly how a module whose
stated contract is "total over nonsense" stopped being one. `has_at()` now
does the same job with the same check as the rest.

Found by re-reading the file after it was already merged and green — the
existing truncation test never produced an `e_lfanew` PAST the end, only one
exactly AT it, where `substr` is well-defined and returns empty.

Author: speak-agent

src/pack/binfmt.cppm

@@ -160,6 +160,19 @@ std::optional<std::uint64_t> le64(std::string_view b, std::size_t off) {
     return v;
 }
 
+// Do the bytes at `off` equal `lit`?
+//
+// NOT `b.substr(off, n) == lit`, and the difference is a crash.
+// `std::string_view::substr` THROWS `std::out_of_range` when `pos > size()`,
+// and `off` here comes from a field READ OUT OF THE FILE — a file starting
+// with "MZ" whose `e_lfanew` is garbage is ordinary malformed input, not a
+// reason to terminate. This module's contract is that it is total over
+// nonsense; one unchecked `substr` was enough to break that promise.
+bool has_at(std::string_view b, std::size_t off, std::string_view lit) {
+    if (off > b.size() || b.size() - off < lit.size()) return false;
+    return b.compare(off, lit.size(), lit) == 0;
+}
+
 // NUL-terminated string at `off`, bounded by the file end.
 std::optional<std::string> cstr(std::string_view b, std::size_t off) {
     if (off >= b.size()) return std::nullopt;
@@ -309,7 +322,7 @@ pe_needed(std::string_view b) {
     auto lfanew = le32(b, 0x3C);
     if (!lfanew) return std::unexpected("PE: no e_lfanew");
     const std::size_t nt = *lfanew;
-    if (b.substr(nt, 4) != std::string_view("PE\0\0", 4))
+    if (!has_at(b, nt, std::string_view("PE\0\0", 4)))
         return std::unexpected("PE: no PE\\0\\0 signature at e_lfanew");
 
     auto numSections = le16(b, nt + 6);
@@ -445,7 +458,7 @@ Ident identify(const std::filesystem::path& binary) {
         // Saying "PE" for a file that has none would send the caller into a
         // parser that cannot succeed.
         if (auto lfanew = detail::le32(b, 0x3C)) {
-            if (b.substr(*lfanew, 4) == std::string_view("PE\0\0", 4)) {
+            if (detail::has_at(b, *lfanew, std::string_view("PE\0\0", 4))) {
                 id.format = Format::Pe;
                 if (auto m = detail::le16(b, *lfanew + 4))
                     id.arch = detail::pe_arch(*m);

tests/unit/test_pack_binfmt.cpp

@@ -324,6 +324,27 @@ TEST(PackBinfmt, ADosStubWithoutAPeSignatureIsNotAPe) {
     EXPECT_FALSE(bf::needed_names(f.path).has_value());
 }
 
+TEST(PackBinfmt, AGarbageELfanewIsRejectedAndDoesNotThrow) {
+    // An "MZ" file whose `e_lfanew` points past the end is ordinary malformed
+    // input: a truncated download, a DOS stub, a text file named `.exe`.
+    //
+    // This crashed. `std::string_view::substr` THROWS `std::out_of_range` when
+    // `pos > size()`, and the offset comes straight out of the file — so
+    // `identify()`, which is documented as never throwing, terminated the
+    // process instead of answering Unknown. Bounds-checked comparison now.
+    for (std::uint32_t lfanew : {0xFFFFFFFFu, 0x7FFFFFFFu, 0x10000u, 0x101u}) {
+        std::string b(0x100, '\0');
+        b[0] = 'M'; b[1] = 'Z';
+        put(b, 0x3C, lfanew, 4);
+        TempFile f{"mzjunk", b};
+        EXPECT_NO_THROW({
+            EXPECT_EQ(bf::identify(f.path).format, bf::Format::Unknown)
+                << "e_lfanew=" << lfanew;
+            EXPECT_FALSE(bf::needed_names(f.path).has_value());
+        }) << "e_lfanew=" << lfanew;
+    }
+}
+
 TEST(PackBinfmt, TruncatedInputIsRejectedRatherThanRead) {
     // Malformed input is ordinary: a half-downloaded file, a text file named
     // `.exe`. Every read is bounds-checked, so the parser is total over it.