From e44f5cb0fa1aa02158e29b295833e1c3c024570e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 7 Jul 2026 19:54:41 -0700 Subject: [PATCH] terminal: guard RefCountedSet lookups against zero-capacity sets Layout.init(0) is an explicitly supported special case that produces a valid zero-capacity set with a zero-size table. But lookupContext had no guard for it: probing computes `table[hash & 0]` and reads whatever memory follows the set in its backing buffer, treating those bytes as an item ID which is then used to index the (also zero-size) items array, an out-of-bounds read reaching arbitrarily far past the set. This has never fired in practice because no production page carries a zero-capacity set today, and where one could occur the adjacent bytes happen to be zero (which reads as an empty bucket and returns null). Page.exactRowCapacity legitimately produces zero capacities for pages without styled or hyperlinked cells though, so any page compaction work makes this reachable with nonzero adjacent memory: in a Page layout the styles set can be followed by the grapheme bitmap, which is initialized to all ones. Lookups on a zero-capacity set now return null without touching the table. This also covers add, which looks up before inserting and already handles the zero capacity correctly after that point by returning OutOfMemory. All other entry points assert on valid IDs, which a zero-capacity set cannot have. --- src/terminal/ref_counted_set.zig | 9 +++++++++ src/terminal/style.zig | 30 ++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/src/terminal/ref_counted_set.zig b/src/terminal/ref_counted_set.zig index 8040039ae..dd76079f4 100644 --- a/src/terminal/ref_counted_set.zig +++ b/src/terminal/ref_counted_set.zig @@ -497,6 +497,15 @@ pub fn RefCountedSet( return self.lookupContext(base, value, self.context); } pub fn lookupContext(self: *const Self, base: anytype, value: T, ctx: Context) ?Id { + // A zero-capacity set (a valid special case of Layout.init) + // contains nothing and has a zero-size table, so we can't + // probe it: table[0] would read whatever memory follows the + // set in the backing buffer. + if (self.layout.table_cap == 0) { + @branchHint(.cold); + return null; + } + const table = self.table.ptr(base); const items = self.items.ptr(base); diff --git a/src/terminal/style.zig b/src/terminal/style.zig index ae9488af8..720789bb2 100644 --- a/src/terminal/style.zig +++ b/src/terminal/style.zig @@ -966,6 +966,36 @@ test "Set capacities" { _ = Set.Layout.init(16384); } +test "Set zero capacity" { + const testing = std.testing; + const alloc = testing.allocator; + + // A zero-capacity set is a valid special case (see Layout.init). + // It occurs in practice for pages with exact capacities where no + // cell is styled (see Page.exactRowCapacity). + const layout: Set.Layout = .init(0); + try testing.expectEqual(0, layout.total_size); + + // We allocate a nonzero buffer filled with 0xFF to simulate the + // set being embedded in a larger structure (e.g. a Page) where + // other data follows it. Lookups must not probe the zero-size + // table: table[0] would read this adjacent memory and treat it + // as an item ID, leading to far out-of-bounds item reads. + const buf = try alloc.alignedAlloc(u8, Set.base_align, 64); + defer alloc.free(buf); + @memset(buf, 0xFF); + + var set = Set.init(.init(buf), layout, .{}); + + const style: Style = .{ .flags = .{ .bold = true } }; + try testing.expectEqual(null, set.lookup(buf, style)); + try testing.expectError(error.OutOfMemory, set.add(buf, style)); + try testing.expectEqual(0, set.count()); + + // The adjacent memory must be untouched. + for (buf) |b| try testing.expectEqual(0xFF, b); +} + test "Style HTML formatting basic bold" { const testing = std.testing; const alloc = testing.allocator;