135d27fa00b4

hook output, an unparseable log line, and nix search descriptions now pass through a new output.sanitize() before reaching the terminal, stripping control bytes < 0x20 (except \n/\t) and CSI sequences so embedded ESC bytes can't inject escape sequences; generalizes exec.zig's stripAnsi (removed, both diff-closures call sites now use output.sanitize) (NINA-016)

time
2026-07-02 14:10 UTC · 3 months ago
machine
koh2
agent
claude-sonnet-5
session
fable audit fix — output.zig
parent
2bc90194d359
4 files src/commands.zig+7 −2src/exec.zig+2 −27src/output.zig+48 −2src/search.zig+26 −2
src/commands.zig+7 −2
@@ -51,7 +51,7 @@5151     switch (hook.result) {5252         .not_found, .ok => return true,5353         .failed => {54-            output.hookFailed(hook_name, hook.exit_code, hook.output);54+            output.hookFailed(ctx.gpa, hook_name, hook.exit_code, hook.output);5555             return output.confirmDefaultNo("continue anyway?");5656         },5757     }@@ -937,7 +937,12 @@937937 // Render ts -> date/time columns, command -> action, outcome(+gen) -> detail.938938 fn renderJsonLogLine(gpa: std.mem.Allocator, line: []const u8) void {939939     const parsed = std.json.parseFromSlice(std.json.Value, gpa, line, .{}) catch {940-        output.logRow(line, "", "", ""); // unparseable — show raw rather than drop940+        // Unparseable — show raw rather than drop, but sanitized: a corrupted or941+        // tampered-with log entry is untrusted text by the time we get here942+        // (NINA-016).943+        const cleaned = output.sanitize(gpa, line) catch line;944+        defer if (cleaned.ptr != line.ptr) gpa.free(cleaned);945+        output.logRow(cleaned, "", "", "");941946         return;942947     };943948     defer parsed.deinit();

view file

src/exec.zig+2 −27
@@ -1602,37 +1602,12 @@16021602     const r = try capture(gpa, io, machine, &.{ "nix", "store", "diff-closures", a_str, b_str });16031603     defer gpa.free(r.stdout);16041604     defer gpa.free(r.stderr);1605-    const cleaned = try stripAnsi(gpa, r.stdout);1605+    const cleaned = try output.sanitize(gpa, r.stdout);16061606     defer gpa.free(cleaned);16071607     output.raw(cleaned);16081608     return r.exit_code;16091609 }16101610 1611-// Strip CSI escape sequences (ESC '[' ... final-byte) from text. Used to clean a1612-// child process's color when it writes ANSI unconditionally.1613-fn stripAnsi(gpa: std.mem.Allocator, input: []const u8) ![]u8 {1614-    var out: std.ArrayList(u8) = .empty;1615-    errdefer out.deinit(gpa);1616-    var i: usize = 0;1617-    while (i < input.len) {1618-        if (input[i] == 0x1b and i + 1 < input.len and input[i + 1] == '[') {1619-            i += 2;1620-            while (i < input.len and !(input[i] >= 0x40 and input[i] <= 0x7e)) : (i += 1) {}1621-            if (i < input.len) i += 1; // consume the final byte1622-            continue;1623-        }1624-        try out.append(gpa, input[i]);1625-        i += 1;1626-    }1627-    return out.toOwnedSlice(gpa);1628-}1629-1630-test "stripAnsi removes CSI color sequences" {1631-    const cleaned = try stripAnsi(std.testing.allocator, "kepr: \x1b[31;1m13.1 KiB\x1b[0m");1632-    defer std.testing.allocator.free(cleaned);1633-    try std.testing.expectEqualStrings("kepr: 13.1 KiB", cleaned);1634-}1635-16361611 // --- Doctor checks ---16371612 16381613 pub fn runDoctorChecks(gpa: std.mem.Allocator, io: std.Io, machine: *const types.Machine) ![]types.DoctorCheck {@@ -1945,7 +1920,7 @@19451920     const r = try capture(gpa, io, machine, &.{ "nix", "store", "diff-closures", a_path, b_path });19461921     defer gpa.free(r.stdout);19471922     defer gpa.free(r.stderr);1948-    const cleaned = try stripAnsi(gpa, r.stdout);1923+    const cleaned = try output.sanitize(gpa, r.stdout);19491924     defer gpa.free(cleaned);19501925     output.raw(cleaned);19511926     return r.exit_code;

view file

src/output.zig+48 −2
@@ -393,7 +393,51 @@393393     p("\n", .{});394394 }395395 396-pub fn hookFailed(name: []const u8, code: u8, hook_output: []const u8) void {396+// Strips terminal-hostile bytes from external/untrusted text before it reaches397+// the terminal: full CSI sequences (ESC '[' ... final byte) — used to scrub a398+// child process's ANSI color when NO_COLOR forces capture-then-print instead of399+// a direct stream — plus any other byte < 0x20 except \n and \t, which would400+// otherwise let hook output, a nix search description, or an unparseable log401+// line smuggle escape sequences (title changes, screen garbage, spoofed lines)402+// into the terminal (NINA-016).403+pub fn sanitize(gpa: std.mem.Allocator, input: []const u8) ![]u8 {404+    var out: std.ArrayList(u8) = .empty;405+    errdefer out.deinit(gpa);406+    var i: usize = 0;407+    while (i < input.len) {408+        const b = input[i];409+        if (b == 0x1b and i + 1 < input.len and input[i + 1] == '[') {410+            i += 2;411+            while (i < input.len and !(input[i] >= 0x40 and input[i] <= 0x7e)) : (i += 1) {}412+            if (i < input.len) i += 1; // consume the final byte413+            continue;414+        }415+        if (b < 0x20 and b != '\n' and b != '\t') {416+            i += 1;417+            continue;418+        }419+        try out.append(gpa, b);420+        i += 1;421+    }422+    return out.toOwnedSlice(gpa);423+}424+425+test "sanitize strips CSI color sequences" {426+    const cleaned = try sanitize(std.testing.allocator, "kepr: \x1b[31;1m13.1 KiB\x1b[0m");427+    defer std.testing.allocator.free(cleaned);428+    try std.testing.expectEqualStrings("kepr: 13.1 KiB", cleaned);429+}430+431+test "sanitize strips bare control bytes but keeps newline and tab" {432+    const cleaned = try sanitize(std.testing.allocator, "line one\x07\nnext\ttab\x1b]0;evil title\x07 done");433+    defer std.testing.allocator.free(cleaned);434+    try std.testing.expectEqualStrings("line one\nnext\ttab]0;evil title done", cleaned);435+}436+437+// gpa sanitizes each hook output line before it reaches the terminal — hooks are438+// the user's own code (weak boundary, but still not nina's own text) and their439+// output could contain embedded escape sequences (NINA-016).440+pub fn hookFailed(gpa: std.mem.Allocator, name: []const u8, code: u8, hook_output: []const u8) void {397441     p("{s}{s} hook exited with code {d}{s}\n\n", .{ hdr(), name, code, c(RESET) });398442 399443     if (hook_output.len > 0) {@@ -411,7 +455,9 @@411455         const start = count - display_count;412456         for (0..display_count) |i| {413457             const line = shown[(start + i) % shown.len];414-            p("   {s}{s}{s}\n", .{ c(DIM), line, c(RESET) });458+            const clean = sanitize(gpa, line) catch line;459+            defer if (clean.ptr != line.ptr) gpa.free(clean);460+            p("   {s}{s}{s}\n", .{ c(DIM), clean, c(RESET) });415461         }416462         p("\n", .{});417463     }

view file

src/search.zig+26 −2
@@ -215,11 +215,35 @@215215         return &.{};216216     }217217     switch (mode) {218-        .packages => return api.searchPackages(arena, io, environ, query) catch &.{},218+        .packages => {219+            const packages: []types.NixPackage = api.searchPackages(arena, io, environ, query) catch &.{};220+            sanitizeDescriptions(arena, packages);221+            return packages;222+        },219223         .options => {220-            opts.* = api.searchOptions(arena, io, query) catch &.{};224+            const options: []api.NixOption = api.searchOptions(arena, io, query) catch &.{};225+            sanitizeOptionDescriptions(arena, options);226+            opts.* = options;221227             return &.{};222228         },229+    }230+}231+232+// nix search's JSON descriptions come from nixpkgs/NixOS module metadata, which233+// upstream authors control — a malicious description could otherwise smuggle234+// escape sequences into both the TUI widget and the plain (piped) renderer,235+// since both read straight from this shared result set (NINA-016). pname/attr236+// are Nix identifiers and version strings are not free text, so only the237+// free-form description needs sanitizing.238+fn sanitizeDescriptions(arena: std.mem.Allocator, packages: []types.NixPackage) void {239+    for (packages) |*pkg| {240+        pkg.description = output.sanitize(arena, pkg.description) catch pkg.description;241+    }242+}243+244+fn sanitizeOptionDescriptions(arena: std.mem.Allocator, options: []api.NixOption) void {245+    for (options) |*opt| {246+        opt.description = output.sanitize(arena, opt.description) catch opt.description;223247     }224248 }225249 

view file