From 507b73e311832587fbfbbee1ff6cbe644110d5f6 Mon Sep 17 00:00:00 2001
From: Hawkynt <github@synthelicz.de>
Date: Tue, 10 Nov 2026 02:10:48 +0100
Subject: [PATCH] # FAT reader no longer recurses forever into cyclic
 directories

Symptom (#805): reading a damaged FAT image killed the process with a stack
overflow (seen on cryptsetup's BitLocker To Go test images, whose zeroed
sectors decrypt to a garbage FAT16).

Root cause: ReadDirectoryEntries descends into every subdirectory entry and
nothing remembered which directories had been read, so a subdirectory whose
start cluster points back at an ancestor - or at itself - was entered again
and again. Cluster chains already had a loop guard; directory recursion did
not.

Fix: track the start clusters of directories already read and skip a
directory that has been visited, keeping the entries read so far.

Verified: new FatCorruptDirectoryTests reproduce the crash (test host
"Stack overflow") before the fix and pass after it, for a parent cycle and a
self cycle; a 20-level valid tree is still listed completely; all 312 FAT
tests pass.

Closes #805
---
 .../Fat/FatCorruptDirectoryTests.cs           | 55 +++++++++++++++++++
 .../FileSystems/FileSystem.Fat/FatReader.cs   |  5 ++
 2 files changed, 60 insertions(+)
 create mode 100644 Compression.Tests/Fat/FatCorruptDirectoryTests.cs

diff --git a/Compression.Tests/Fat/FatCorruptDirectoryTests.cs b/Compression.Tests/Fat/FatCorruptDirectoryTests.cs
new file mode 100644
index 000000000..6c6d8b9c7
--- /dev/null
+++ b/Compression.Tests/Fat/FatCorruptDirectoryTests.cs
@@ -0,0 +1,55 @@
+using System.Buffers.Binary;
+using System.Text;
+
+namespace Compression.Tests.Fat;
+
+/// <summary>
+/// Damaged directory trees must not take the reader down: a subdirectory whose start cluster
+/// points back at an ancestor (or at itself) would otherwise be descended into forever until the
+/// stack overflows and kills the process (#805).
+/// </summary>
+[TestFixture]
+public class FatCorruptDirectoryTests {
+
+  /// <summary>A FAT image with docs/inner/deep.txt, where inner's entry is then pointed at <paramref name="target"/>'s cluster.</summary>
+  private static byte[] CyclicImage(string target) {
+    var w = new FileSystem.Fat.FatWriter();
+    w.AddFile("readme.txt", "root"u8.ToArray());
+    w.AddFile("docs/guide.txt", "guide"u8.ToArray());
+    w.AddFile("docs/inner/deep.txt", "deep"u8.ToArray());
+    var disk = w.Build();
+
+    int docsCluster, innerCluster;
+    using (var r = new FileSystem.Fat.FatReader(new MemoryStream(disk))) {
+      docsCluster = r.Entries.Single(e => e.Name == "docs").StartCluster;
+      innerCluster = r.Entries.Single(e => e.Name == "docs/inner").StartCluster;
+    }
+    var name = Encoding.ASCII.GetBytes("INNER      ");
+    var patched = 0;
+    for (var off = 0; off + 32 <= disk.Length; off += 32)
+      if (disk.AsSpan(off, 11).SequenceEqual(name) && (disk[off + 11] & 0x10) != 0) {
+        BinaryPrimitives.WriteUInt16LittleEndian(disk.AsSpan(off + 26), (ushort)(target == "self" ? innerCluster : docsCluster));
+        ++patched;
+      }
+    Assert.That(patched, Is.EqualTo(1), "the inner directory entry should be found exactly once");
+    return disk;
+  }
+
+  [TestCase("parent")]
+  [TestCase("self")]
+  public void GivenADirectoryPointingBackIntoItsOwnAncestry_WhenRead_ThenTheReaderTerminatesWithTheIntactEntries(string target) {
+    using var r = new FileSystem.Fat.FatReader(new MemoryStream(CyclicImage(target)));
+    var names = r.Entries.Select(e => e.Name).ToList();
+    Assert.That(names, Does.Contain("readme.txt").And.Contain("docs/guide.txt").And.Contain("docs/inner"));
+    Assert.That(names.Count, Is.LessThan(50), "the cycle must not be expanded");
+  }
+
+  [Test]
+  public void GivenADeepButValidTree_WhenRead_ThenEveryLevelIsStillListed() {
+    var w = new FileSystem.Fat.FatWriter();
+    var path = string.Join('/', Enumerable.Range(0, 20).Select(i => $"d{i}"));
+    w.AddFile(path + "/leaf.txt", "leaf"u8.ToArray());
+    using var r = new FileSystem.Fat.FatReader(new MemoryStream(w.Build()));
+    Assert.That(r.Entries.Any(e => e.Name == path + "/leaf.txt"), Is.True);
+  }
+}
diff --git a/Hawkynt.FileFormats.FileSystems/FileSystems/FileSystem.Fat/FatReader.cs b/Hawkynt.FileFormats.FileSystems/FileSystems/FileSystem.Fat/FatReader.cs
index 5a408b030..fc347d30a 100644
--- a/Hawkynt.FileFormats.FileSystems/FileSystems/FileSystem.Fat/FatReader.cs
+++ b/Hawkynt.FileFormats.FileSystems/FileSystems/FileSystem.Fat/FatReader.cs
@@ -102,7 +102,12 @@ private void Parse() {
     }
   }
 
+  // Start clusters of the directories already read. A damaged tree can point a subdirectory back
+  // at an ancestor or at itself; descending into it again would recurse until the stack overflows.
+  private readonly HashSet<int> _visitedDirectories = [];
+
   private void ReadDirectory(int cluster, string path) {
+    if (!_visitedDirectories.Add(cluster)) return;
     var clusterData = ReadClusterChain(cluster);
     var entryCount = clusterData.Length / 32;
     ReadDirectoryEntries(clusterData, entryCount, path);