Skip to content

ntfs: bound the MFT; exfat: clamp the boot cluster count - #26

Merged
switch-nz merged 4 commits into
switch-nz:mainfrom
deadp:ntfs-mft-bounds
Sep 17, 2026
Merged

switch-nz merged 4 commits into
switch-nz:mainfrom
deadp:ntfs-mft-bounds

Conversation

@deadp

@deadp deadp commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Stage 3 of the #15 fuzzer findings, plus the maintainer follow-up on #17.

NTFS — item 3 (hang + AttributeError)

  • When record 0's $DATA cannot be read, the fallback $MFT run was (mft_cluster, 1 << 20) regardless of volume size — the fuzzer's hang input then claimed 536,870,912 records and build_tree() (every listdir) read them all. The fallback run is now clamped to cluster_count, and the computed record_count is clamped to source_size // record_size with a finding when that fires.
  • record() cached the parsed record before checking rec.valid, so an unformatted record was None on first lookup and an invalid MftRecord (no .sequence, stat() → AttributeError at ntfs.py:699) on every later one. Only valid records enter the cache now; cache hits need no re-check.
  • The boot sector's record size is floored at 128 bytes: degenerate values (< 128, e.g. the hang image's 2) fall back to the usual 1024; every plausible real size (128–65536) is untouched.
  • Flips test_record_count_bounded_by_volume and test_invalid_record_stays_invalid_on_second_lookup (both were @unittest.expectedFailure bug markers).

exFAT — item 1 residual (#17 follow-up)

The #17 heap-end clamp in chain() trusts cluster_count itself, so a boot sector claiming 0xFFFFFFFF clusters (offset 92) still let a NoFatChain stream expand by the billions — the reviewer's mutation of the saved #17 input mounted and hung a stat loop. The boot sector's cluster count is now bounded by what the image actually holds past the cluster heap (with a finding), which protects both the contiguous limit and the FAT-walk bound.

Verification

  • Full suite: Ran 305, OK (expected failures=14) (baseline 304/16: −2 flips, +1 new test); tests/smoke.py exit 0.
  • All three saved fuzz inputs replay clean (ntfs-hang-3ebda510.bin previously exit 124 under timeout 30; ntfs-exception-b9407023.bin previously AttributeError; exfat-memory-4c12571f.bin unchanged).
  • New test_cluster_count_bounded_by_image pins the exFAT clamp; post-fix probe of the hang image: record_size == 1024, record_count == 32 ≤ 128, record(30) → None on repeat lookups, stat({"mft": 30}) → {}.

Fixes part of #15.

deadp and others added 4 commits September 17, 2026 16:26
When record 0's $DATA cannot be read, the fallback $MFT run was
(4, 1 << 20) regardless of volume size, so record_count could claim
over half a billion records and build_tree() (every listdir) read them
all: the fuzzer's hang. Clamp the fallback run to cluster_count, clamp
the computed record_count to source_size // record_size (with a
finding when it fires), and floor the boot sector's record size at 128
bytes so a degenerate value falls back to the usual 1024.

record() also cached the parsed record before checking rec.valid, so an
unformatted record was None on first lookup and an invalid MftRecord
(no .sequence) on every later one: the fuzzer's AttributeError in
stat(). Only valid records enter the cache now.

Flips test_record_count_bounded_by_volume and
test_invalid_record_stays_invalid_on_second_lookup.

Fixes part of switch-nz#15.
The switch-nz#17 heap-end clamp in chain() trusts cluster_count itself, so a
boot sector claiming 0xFFFFFFFF clusters (offset 92) still let a
NoFatChain stream expand by the billions: the fuzzer's stat loop hung
on the mutated image even after that fix. Bound the boot sector's
cluster count by what the image actually holds past the cluster heap
(with a finding), which protects both the contiguous limit and the
FAT-walk bound.

Fixes part of switch-nz#15.
Adds the Unreleased entries CONTRIBUTING.md asks for, and brings main
into the branch so they sit with the jbd2 entry above 0.1.1.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@switch-nz
switch-nz merged commit 0ea5451 into switch-nz:main Sep 17, 2026
26 checks passed
switch-nz added a commit to deadp/strata that referenced this pull request Sep 17, 2026
Resolves the CHANGELOG conflict with switch-nz#26 by keeping both sets of entries.
The exFAT timestamp entry now says that an entry recording no valid
offset is still shown unadjusted and labelled UTC, so examiners know to
read those times as the device's local time.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@switch-nz

Copy link
Copy Markdown
Owner

Merged — thanks, Josh. Reviewed and verified:

  • Clamps are live on real volumes, not just in tests. Both read source.size; the server hands filesystems an OffsetReader (partition size) or, for BitLocker/LUKS, a DecryptedReader — both set size.
  • exFAT, reproduced on both sides using the exfat: bound contiguous run expansion at cluster heap end #17 mutation plus a 2^50 stream length, with list stubbed to count without allocating: old code requested 4,294,967,287 items for cluster_count=0xFFFFFFFF (268,435,448 for 2^28); this change requests 56 and records the finding. A genuine boot sector is unchanged.
  • NTFS, reproduced: on the old engine the 4 KB image claimed 4,194,304 MFT records, and an invalid record came back as a live MftRecord on the second lookup; both pass here.
  • Fuzz: 112,485 cases on the original head with no NTFS or exFAT finding, and 133,814 cases with zero findings once merged with main (which has jbd2: keep an invalid journal safely answerable #25).

Pushed before merging: main merged in, plus Unreleased changelog entries for the NTFS and exFAT fixes.

One residual worth knowing, not a blocker. The exFAT bound is now the image's real size, which is right, but chain() still materialises a contiguous run as one list element per cluster. On a large genuine volume a single corrupt stream length can still ask for a list as long as the volume has clusters — millions at typical exFAT cluster sizes. Representing contiguous runs as (start, count), as #15 originally suggested, would remove that class entirely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants