From 02d70c8eac9b413ce47d5b1c25e50ce40d1f96ac Mon Sep 17 00:00:00 2001 From: Evan Sultanik Date: Fri, 18 Sep 2026 16:47:47 -0400 Subject: [PATCH] Resolve an indirect offset inside a named list from the use site A test nested under a `name` block counts its offsets from wherever the `use` that dispatched it matched. `NamedAbsoluteOffset` does that, but only for a test whose own offset is an `AbsoluteOffset`. An indirect offset is not one, so the position it reads its pointer from was left counting from the start of the file. Every `(N.x)` inside a named list therefore read the wrong bytes. On a Mach-O universal binary, `magic_defs/cafebabe:29` declares `>(8.L) indirect x`, which should read the architecture's file offset 8 bytes into the fat_arch record the `use` landed on. It read absolute offset 8 instead, which is the CPU type: /usr/bin/nohup pointer at absolute 16 = 16384 the first architecture pointer at absolute 36 = 49152 the second what was read = 16777223 0x01000007, CPU_TYPE_X86_64 Both architectures dispatched to the same out-of-bounds offset, so neither reported anything and the brackets came out empty: file: ... [x86_64:\012- Mach-O 64-bit x86_64 executable, flags:<...>] ... before: ... [ x86_64:] [ arm64e (caps: 0x2):] after: ... [ x86_64: Mach-O 64-bit x86_64 executable, flags:<...>] ... `rebase_in_named_test` walks the offset instead of type-testing it. It rebases a position and leaves a distance alone: what a relative (`&`) offset wraps is a distance from the previous match, and rebasing it would add the `use` site twice. Two differences from `file` remain on this input, both in how the message is assembled rather than where it reads: PolyFile writes `[ x86_64` where libmagic writes `[x86_64`, and libmagic emits its match separator before each nested verdict. Over 2,000 real files this fix alone moves the strongest-match agreement by one file, because the Mach-O descriptions now hold the right content but still do not match byte for byte. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01RG45aqyyCLkL5b1UB5GWd7 --- polyfile/magic.py | 43 ++++++++++++++++++++++++++++++++++++-- tests/test_magic.py | 51 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 2 deletions(-) diff --git a/polyfile/magic.py b/polyfile/magic.py index 1b386cf..59ae98c 100644 --- a/polyfile/magic.py +++ b/polyfile/magic.py @@ -813,6 +813,45 @@ def __str__(self): return f"({self.offset!s}{['.', ','][self.signed]}{num_bytes}{self.endianness.value})" +def rebase_in_named_test(offset: Offset, named_test: "NamedTest") -> Offset: + """Resolves the positions in `offset` against the offset a named test was invoked at. + + A test nested under a ``name`` block counts its offsets from wherever the ``use`` that + dispatched it matched, not from the start of the file. `NamedAbsoluteOffset` does that for a + test whose own offset is absolute, and this reaches the absolute offsets underneath one that + is not. + + An indirect offset reads its pointer at a position, so that position is rebased too. A + relative (``&``) offset holds a *distance* from the previous match rather than a position, so + what it wraps is left alone: rebasing a distance would add the ``use`` site to it twice. + + Args: + offset: The offset as parsed, counted from the start of the file. + named_test: The ``name`` block the test carrying `offset` belongs to. + + Returns: + The offset with every position it reads resolved against the ``use`` site. The argument is + returned unchanged when it holds no absolute position. + """ + if isinstance(offset, NamedAbsoluteOffset): + return offset + if isinstance(offset, AbsoluteOffset): + return NamedAbsoluteOffset(named_test, offset.offset) + if isinstance(offset, IndirectOffset): + rebased = rebase_in_named_test(offset.offset, named_test) + if rebased is offset.offset: + return offset + return IndirectOffset( + offset=rebased, + num_bytes=offset.num_bytes, + endianness=offset.endianness, + signed=offset.signed, + post_process=offset.post_process, + is_id3=offset.is_id3, + ) + return offset + + INDIRECT_OFFSET_TYPES: Dict[Tuple[int, Endianness], str] = { (1, Endianness.LITTLE): "byte", (1, Endianness.BIG): "byte", (2, Endianness.LITTLE): "leshort", (2, Endianness.BIG): "beshort", @@ -1110,8 +1149,8 @@ def __init__( self.level: int = self.parent.level + 1 parent.children.append(self) self.named_test: Optional[NamedTest] = parent.named_test - if self.named_test is not None and isinstance(offset, AbsoluteOffset): - self.offset = NamedAbsoluteOffset(self.named_test, offset.offset) + if self.named_test is not None: + self.offset = rebase_in_named_test(offset, self.named_test) if mime is not None: parent.can_match_mime = True else: diff --git a/tests/test_magic.py b/tests/test_magic.py index 38245a0..17cdcf0 100644 --- a/tests/test_magic.py +++ b/tests/test_magic.py @@ -2051,6 +2051,57 @@ def test_an_env_python_script_is_described(self): self.assertNotIn("Python script text executable", messages) +class NamedTestOffsetTest(TestCase): + """An offset inside a `name` block counts from wherever the `use` dispatched it. + + `NamedAbsoluteOffset` does that for a test whose own offset is absolute. An indirect offset + reads its pointer at a position too, and that position was left counting from the start of the + file, so every `(N.x)` inside a named list read the wrong bytes. On a Mach-O universal binary + that meant reading the CPU type where the architecture's file offset should be, and reporting + `[x86_64:]` with nothing inside the brackets. + """ + + POINTER_IN_NAMED_LIST: str = "\n".join(( + "0\tname\tblk\t\\b [", + ">(4.L)\tindirect\tx\t\\b:", + "", + "0\tstring\tMAGI\tbase", + ">8\tuse\tblk\t\\b", + "", + "0\tstring\tNESTED\tnested", + "", + )) + """`blk` is dispatched at offset 8, so its `(4.L)` reads the pointer at offset 12.""" + + @staticmethod + def sample() -> bytes: + """A file whose pointer at offset 12 leads to `NESTED`, with a decoy at offset 4. + + The decoy is what an offset counted from the start of the file would read instead. + """ + data = bytearray(b"\x00" * 48) + data[0:4] = b"MAGI" + data[4:8] = struct.pack(">I", 44) + data[12:16] = struct.pack(">I", 32) + data[32:38] = b"NESTED" + return bytes(data) + + def messages(self, definitions: str, data: bytes) -> Set[str]: + """Matches `data` against ad-hoc definitions written to a temporary file.""" + with TemporaryDirectory() as directory: + path = Path(directory) / "definitions" + path.write_text(definitions) + return {str(match) for match in MagicMatcher.parse(path).match(data)} + + def test_an_indirect_offset_reads_from_the_use_site(self): + """`file` 5.48 reports `base [:nested` for this input.""" + messages = self.messages(self.POINTER_IN_NAMED_LIST, self.sample()) + self.assertTrue( + any("nested" in message for message in messages), + f"the named list's indirect offset did not reach the pointer it declares: {messages!r}", + ) + + class UseTestSemanticsTest(TestCase): """Regression tests for the `use` test truth value reported in issue #3484."""