From b244567faadf24bee993bce98017e65ff4c383f6 Mon Sep 17 00:00:00 2001 From: ddidderr Date: Mon, 12 Oct 2026 15:56:21 +0200 Subject: [PATCH] fix-deviant(os): register and test the NOTIFY_SOCKET bounds fix Intentional deviation from Ghostty 1.3.1 (DEVIANT_FIXES.md row 33). Zig's os/systemd.zig:116 accepts a NOTIFY_SOCKET path of exactly 108 bytes and then writes the terminating NUL to path[108], one past the end of sockaddr_un.sun_path (out-of-bounds write). The Rust port's socket address construction (since f8c5ce0) accepts at most 108 bytes without a NUL and rejects longer paths, so it never writes out of bounds; this commit records that deviation and adds a regression test. Test Plan: - cargo test -p ghostty-os notify_maximum_length_socket_path_is_rejected_safely --- crates/ghostty-os/src/systemd.rs | 33 ++++++++++++++++++++++++++++++++ docs/rewrite/DEVIANT_FIXES.md | 1 + 2 files changed, 34 insertions(+) diff --git a/crates/ghostty-os/src/systemd.rs b/crates/ghostty-os/src/systemd.rs index 11fa787..c0ac74a 100644 --- a/crates/ghostty-os/src/systemd.rs +++ b/crates/ghostty-os/src/systemd.rs @@ -242,4 +242,37 @@ mod tests { temp_env::with_var("NOTIFY_SOCKET", Some(""), notify::ready); temp_env::with_var("NOTIFY_SOCKET", Some("relative"), notify::ready); } + + /// Upstream bug (`os/systemd.zig:116`): a 108-byte `NOTIFY_SOCKET` path + /// passes Zig's length check (`108 < len`) and then writes the + /// terminating NUL at `path[108]`, one past the 108-byte array. Rust's + /// socket-address constructor builds the address from the 108 bytes + /// without a terminator (valid for Linux `AF_UNIX`; the length is part of + /// the address), so there is no out-of-bounds write; longer paths are + /// rejected. + #[test] + fn notify_maximum_length_socket_path_is_rejected_safely() { + let mut max = vec![b'/'; 108]; + max[1..].fill(b'a'); + // 108 bytes fit exactly (no terminator needed); 109 do not. + assert!(rustix::net::SocketAddrUnix::new(&max[..107]).is_ok()); + assert!(rustix::net::SocketAddrUnix::new(&max[..108]).is_ok()); + let mut over = max.clone(); + over.push(b'a'); + assert!(rustix::net::SocketAddrUnix::new(&over[..]).is_err()); + + for len in [107usize, 108, 109, 4096] { + let mut p = vec![b'a'; len]; + p[0] = b'/'; + let p = String::from_utf8(p).unwrap(); + // No panic and no out-of-bounds write (nothing listens on these + // paths, so the send just fails or is skipped). + temp_env::with_var("NOTIFY_SOCKET", Some(p.as_str()), notify::ready); + } + // '@' + 107 name bytes = 108 bytes. + let mut abs = vec![b'a'; 108]; + abs[0] = b'@'; + let abs = String::from_utf8(abs).unwrap(); + temp_env::with_var("NOTIFY_SOCKET", Some(abs.as_str()), notify::ready); + } } diff --git a/docs/rewrite/DEVIANT_FIXES.md b/docs/rewrite/DEVIANT_FIXES.md index 779f7b0..e90bfca 100644 --- a/docs/rewrite/DEVIANT_FIXES.md +++ b/docs/rewrite/DEVIANT_FIXES.md @@ -58,6 +58,7 @@ when this file was written. | 30 | CLI: stdout as a regular file | `std.fs.File.writer` (Zig 0.15.2) as used in `cli/show_face.zig:68-73`, `cli/help.zig:34`, `cli/validate_config.zig:43-46`, `cli/show_config.zig:84` | The writer `pwrite`s at its own offset starting at 0: with stdout redirected to a file, output overwrites itself and earlier file content, and `Writer.end` truncates. | Writes sequentially at the inherited descriptor's current offset (no `pwrite` from offset 0, so earlier content and the writers' own output are not overwritten; this appends only when the descriptor is opened for append, e.g. `>>`); no truncation. | `410d8fc` fix-deviant(cli) (append behaviour first in `f76179f`) | `zig_writer::tests::regular_file_output_is_complete_and_appended` | | 31 | GTK: `+new-window` D-Bus arguments | `apprt/gtk/ipc/new_window.zig:49-53` (`Bytes.new(ptr, len + 1)` + `Variant.newFromBytes(..., trusted = true)`) | Arguments are passed as trusted GVariant strings without validation; invalid UTF-8 or an interior NUL makes GIO abort the process while serializing (input-triggered crash). | `arguments_variant` validates first and returns `Error::IpcFailed` (message on stderr, no D-Bus call); valid arguments are byte-identical to Zig. | `65dd9d5` fix-deviant(gtk) | `ipc::new_window::tests::invalid_dbus_strings_are_rejected_without_abort` (`ghostty-apprt-gtk`) | | 32 | OS: URL opener child reaping | `os/open.zig` (`try collectOutput(...)` before `wait()`) | When collecting the opener's output fails (e.g. more than 50 KiB on a stream), the error returns before `wait()`, leaving the child as a zombie. | The child is always reaped; the collection error is still logged. | `4a55abf` fix-deviant(os) | `ghostty-os` open tests | +| 33 | OS: maximum-length `NOTIFY_SOCKET` | `os/systemd.zig:100-116` (`socket_address.path.len < socket_path.len`, then `path[socket_path.len] = 0`) | A 108-byte path (the size of `sockaddr_un.path`) passes the length check, and the terminating NUL is written at `path[108]`, one byte past the array (out-of-bounds write). Separate from row 13 (the abstract-socket `@` handling). | The address is built with rustix's `SocketAddrUnix::new`, which takes the 108 bytes without a terminator (valid for Linux `AF_UNIX`) and rejects longer paths; no out-of-bounds write. Verified in code. | f8c5ce0 (`git log -S` on `ghostty-os/src/systemd.rs`: the only commit touching it, besides formatting/macOS changes) | `notify_maximum_length_socket_path_is_rejected_safely` (`ghostty-os/src/systemd.rs`) | ## Upstream quirks deliberately kept (not fixed)