a73x

45b260de

fix(shellint): shim directory created exclusively, named unaimably

a73x   2026-08-14 12:45

Commit message
fix(shellint): shim directory created exclusively, named unaimably

Review finding (Major, CWE-59): the shim lived at the guessable
<sockdir>/mux-shellint-<pid>, and xdg.makePrivateDir built it with
makePath (adopts a pre-existing entry) then openDir + chmod (both follow
symlinks). With XDG_RUNTIME_DIR unset the sockdir is shared /tmp, so a
local user could plant a symlink at the pid name and have the daemon
re-mode a directory of their choosing to 0700 and truncating-create
shell rc files inside it. The benign face of the same defect: a
leftover from a SIGKILLed predecessor at the pid name silently cost the
new session its marks.

New xdg.makeNewPrivateDir: a single posix.mkdir(dir, 0o700) — which
neither follows a symlink nor adopts an existing entry — mapping
PathAlreadyExists to error.DirExists, then a no_follow open; the mode
rides the mkdir so the directory is never briefly wider. makePrivateDir
keeps its adopting behavior for the two callers under ~ where adopting
an existing ~/.config/mux is correct; both doc comments now name their
threat models. The shim name gains 12 random hex
(mux-shellint-<pid>-<random>), so the path cannot be aimed at in
advance and a pid collision no longer exists to degrade. EEXIST still
degrades to no-marks, never fatal; cleanup is untouched because the
exact created path travels in Injection.dir.

Tests: a planted-symlink trespass test (refusal writes nothing through
the link, re-modes nothing, deletes nothing it did not create); the
three pins of the old behavior updated deliberately — the server-level
collision test's stated premise ("the path is knowable in advance") was
precisely the vulnerability, so it now asserts the daemon claims its
suffixed directory and the planted file survives teardown.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
src/server.zig
Old New
@@ -5239,7 +5239,7 @@ test "Server: the shim directory is private, and teardown takes it with it" {
5239 try std.testing.expectError(error.FileNotFound, std.fs.cwd().access(dir, .{})); 5239 try std.testing.expectError(error.FileNotFound, std.fs.cwd().access(dir, .{}));
5240 } 5240 }
5241 5241
5242 test "Server: a shim directory that cannot be created costs the marks, not the session" { 5242 test "Server: an entry already at the daemon's pid name costs neither the marks nor itself" {
5243 const alloc = std.testing.allocator; 5243 const alloc = std.testing.allocator;
5244 std.fs.cwd().access("/bin/bash", .{}) catch return error.SkipZigTest; 5244 std.fs.cwd().access("/bin/bash", .{}) catch return error.SkipZigTest;
5245 5245
@@ -5248,11 +5248,10 @@ test "Server: a shim directory that cannot be created costs the marks, not the s
5248 const sock_path = try std.fmt.allocPrint(alloc, "{s}/degrade.sock", .{tmp.path()}); 5248 const sock_path = try std.fmt.allocPrint(alloc, "{s}/degrade.sock", .{tmp.path()});
5249 defer alloc.free(sock_path); 5249 defer alloc.free(sock_path);
5250 5250
5251 // Plant a regular FILE exactly where init will want its shim directory. 5251 // Plant a regular FILE at the pid-derived name, which is what a daemon
5252 // The path is knowable in advance because init names it after the 5252 // SIGKILLed before teardown leaves behind and what a stranger with write
5253 // daemon's pid, and in a test the daemon IS this process — which is also 5253 // access to a shared /tmp would aim at. In a test the daemon IS this
5254 // the realistic field version of this: a SIGKILLed daemon leaves an entry 5254 // process, so the pid half of the name is exactly the one init draws.
5255 // behind and a later daemon draws the same pid.
5256 const planted = try std.fmt.allocPrint( 5255 const planted = try std.fmt.allocPrint(
5257 alloc, 5256 alloc,
5258 "{s}/mux-shellint-{d}", 5257 "{s}/mux-shellint-{d}",
@@ -5261,17 +5260,19 @@ test "Server: a shim directory that cannot be created costs the marks, not the s
5261 defer alloc.free(planted); 5260 defer alloc.free(planted);
5262 try std.fs.cwd().writeFile(.{ .sub_path = planted, .data = "not a directory" }); 5261 try std.fs.cwd().writeFile(.{ .sub_path = planted, .data = "not a directory" });
5263 5262
5264 // The daemon still starts. This is the whole claim: marks are an
5265 // enhancement, and a session that cannot have them is still a session.
5266 // (init prints one line to stderr on the way past; that is the point of
5267 // it not being silent.)
5268 // Scoped so deinit runs before the survival check below. 5263 // Scoped so deinit runs before the survival check below.
5269 { 5264 {
5270 var srv = try Server.init(alloc, .{ .sock_path = sock_path, .shell = "/bin/bash" }); 5265 var srv = try Server.init(alloc, .{ .sock_path = sock_path, .shell = "/bin/bash" });
5271 defer srv.deinit(); 5266 defer srv.deinit();
5272 5267
5273 // Nothing to tear down later, because nothing was created. 5268 // The shim name carries a random suffix past the pid, so the planted
5274 try std.testing.expectEqual(@as(?[]const u8, null), srv.shellint_dir); 5269 // entry is not in the way of anything: this session gets its marks.
5270 // Before that suffix existed, a collision here cost the session its
5271 // marks for the lifetime of the daemon.
5272 const dir = srv.shellint_dir orelse return error.NoShimDirectory;
5273 try std.testing.expect(std.mem.startsWith(u8, dir, planted));
5274 try std.testing.expect(dir.len > planted.len);
5275 try std.fs.cwd().access(dir, .{});
5275 5276
5276 // ...and the session actually works. A daemon that starts and then 5277 // ...and the session actually works. A daemon that starts and then
5277 // cannot answer would satisfy every assertion above. 5278 // cannot answer would satisfy every assertion above.
@@ -5284,14 +5285,11 @@ test "Server: a shim directory that cannot be created costs the marks, not the s
5284 defer f.deinit(alloc); 5285 defer f.deinit(alloc);
5285 const st = try proto.decodeStatusReply(f.payload); 5286 const st = try proto.decodeStatusReply(f.payload);
5286 try std.testing.expectEqual(@as(u16, 80), st.cols); 5287 try std.testing.expectEqual(@as(u16, 80), st.cols);
5287 // pgid, not marks: the session is honestly running on the fallbacks.
5288 try std.testing.expectEqual(proto.Mechanism.pgid, st.cmd.mechanism);
5289 } 5288 }
5290 5289
5291 // The planted file is still there. Teardown deletes the shim tree by 5290 // The planted file is untouched — not adopted, not written through, and
5292 // path, so a version that recorded the directory even when prepare 5291 // not deleted by the teardown of a shim it was never part of. In the
5293 // failed would delete something this daemon never created — and in the 5292 // field that entry belongs to whatever else drew the pid.
5294 // field that path belongs to whatever else drew the pid.
5295 const kept = try std.fs.cwd().readFileAlloc(alloc, planted, 64); 5293 const kept = try std.fs.cwd().readFileAlloc(alloc, planted, 64);
5296 defer alloc.free(kept); 5294 defer alloc.free(kept);
5297 try std.testing.expectEqualStrings("not a directory", kept); 5295 try std.testing.expectEqualStrings("not a directory", kept);
src/shellint.zig
Old New
@@ -168,10 +168,23 @@ pub const no_injection: Injection = .{ .extra_argv = &.{}, .env = &.{}, .dir = n
168 /// a silent fallback here would look exactly like a shell that ignores 168 /// a silent fallback here would look exactly like a shell that ignores
169 /// its rc. 169 /// its rc.
170 /// 170 ///
171 /// The `mux-shellint-<pid>` naming lives here rather than at the call 171 /// The `mux-shellint-<pid>-<random>` naming lives here rather than at the
172 /// site: it is the same fact as what `prepare` writes and what the 172 /// call site: it is the same fact as what `prepare` writes and what the
173 /// returned `dir` promises to delete, and the pid is what keeps two 173 /// returned `dir` promises to delete, and the pid is what keeps two
174 /// daemons sharing one runtime directory out of each other's shims. 174 /// daemons sharing one runtime directory legible in a directory listing.
175 ///
176 /// The random half is not decoration. `parent_dir` is the socket's
177 /// directory, which is `$XDG_RUNTIME_DIR` when there is one and a shared
178 /// `/tmp` when there is not — and a pid is guessable. On the `/tmp` box
179 /// another user could pre-create the exact name this daemon was going to
180 /// pick, as a symlink to a directory of ours, and the shim files (which
181 /// the session shell then SOURCES) would land through it. `prepare`
182 /// creating the directory exclusively is what closes that; the random
183 /// half is what keeps the attempt from being cheap to aim.
184 ///
185 /// It also fixes the mundane version of the same collision: a predecessor
186 /// SIGKILLed before teardown leaves `mux-shellint-<its pid>` behind, and a
187 /// later daemon drawing that pid used to lose its marks to the leftover.
175 pub fn install( 188 pub fn install(
176 arena: std.mem.Allocator, 189 arena: std.mem.Allocator,
177 parent_dir: []const u8, 190 parent_dir: []const u8,
@@ -179,8 +192,8 @@ pub fn install(
179 ) Injection { 192 ) Injection {
180 const dir = std.fmt.allocPrint( 193 const dir = std.fmt.allocPrint(
181 arena, 194 arena,
182 "{s}/mux-shellint-{d}", 195 "{s}/mux-shellint-{d}-{x:0>12}",
183 .{ parent_dir, std.os.linux.getpid() }, 196 .{ parent_dir, std.os.linux.getpid(), std.crypto.random.int(u48) },
184 ) catch { 197 ) catch {
185 std.debug.print( 198 std.debug.print(
186 "muxd: shell integration unavailable (out of memory naming the shim " ++ 199 "muxd: shell integration unavailable (out of memory naming the shim " ++
@@ -203,6 +216,12 @@ pub fn install(
203 /// and return what spawn must add. All returned slices are allocated from 216 /// and return what spawn must add. All returned slices are allocated from
204 /// `arena` — hand it an arena that lives as long as the daemon. 217 /// `arena` — hand it an arena that lives as long as the daemon.
205 /// 218 ///
219 /// `dir` is created EXCLUSIVELY, so everything written below it is written
220 /// through path components this process made: an entry already at `dir` is
221 /// `error.DirExists` and the session does without marks. Nothing here may
222 /// adopt a directory it did not create — see xdg.makeNewPrivateDir for why
223 /// the difference is a symlink attack and not a preference.
224 ///
206 /// `install` is what the daemon calls; this stays public for the tests, 225 /// `install` is what the daemon calls; this stays public for the tests,
207 /// which need to name their own directory. 226 /// which need to name their own directory.
208 pub fn prepare( 227 pub fn prepare(
@@ -212,7 +231,7 @@ pub fn prepare(
212 ) !Injection { 231 ) !Injection {
213 switch (detect(shell_path)) { 232 switch (detect(shell_path)) {
214 .zsh => { 233 .zsh => {
215 try xdg.makePrivateDir(dir); 234 try xdg.makeNewPrivateDir(dir);
216 // An injection either lands whole or leaves nothing for 235 // An injection either lands whole or leaves nothing for
217 // teardown to guess about: `dir` only reaches `Injection.dir` 236 // teardown to guess about: `dir` only reaches `Injection.dir`
218 // on the success return below, so any failure between here and 237 // on the success return below, so any failure between here and
@@ -235,7 +254,7 @@ pub fn prepare(
235 return .{ .extra_argv = &.{}, .env = try env.toOwnedSlice(arena), .dir = dir }; 254 return .{ .extra_argv = &.{}, .env = try env.toOwnedSlice(arena), .dir = dir };
236 }, 255 },
237 .bash => { 256 .bash => {
238 try xdg.makePrivateDir(dir); 257 try xdg.makeNewPrivateDir(dir);
239 // See the zsh arm above: an injection either lands whole or 258 // See the zsh arm above: an injection either lands whole or
240 // leaves nothing for teardown to guess about. 259 // leaves nothing for teardown to guess about.
241 errdefer std.fs.cwd().deleteTree(dir) catch {}; 260 errdefer std.fs.cwd().deleteTree(dir) catch {};
@@ -248,13 +267,19 @@ pub fn prepare(
248 return .{ .extra_argv = argv, .env = &.{}, .dir = dir }; 267 return .{ .extra_argv = argv, .env = &.{}, .dir = dir };
249 }, 268 },
250 .fish => { 269 .fish => {
251 const vendor = try std.fs.path.join(arena, &.{ dir, "fish", "vendor_conf.d" }); 270 // The root first and on its own, because it is the only
252 try xdg.makePrivateDir(vendor); 271 // component whose parent is a directory strangers can write
272 // to: once it exists, 0700 and ours, the two levels below it
273 // are being created somewhere nobody else can reach.
274 try xdg.makeNewPrivateDir(dir);
253 // `dir`, not `vendor`: it's the root `Injection.dir` would have 275 // `dir`, not `vendor`: it's the root `Injection.dir` would have
254 // named below, and it's what teardown would otherwise be left 276 // named below, and it's what teardown would otherwise be left
255 // to guess about — deleteTree on it takes the nested vendor 277 // to guess about — deleteTree on it takes the nested vendor
256 // directory with it. 278 // directory with it.
257 errdefer std.fs.cwd().deleteTree(dir) catch {}; 279 errdefer std.fs.cwd().deleteTree(dir) catch {};
280 try xdg.makeNewPrivateDir(try std.fs.path.join(arena, &.{ dir, "fish" }));
281 const vendor = try std.fs.path.join(arena, &.{ dir, "fish", "vendor_conf.d" });
282 try xdg.makeNewPrivateDir(vendor);
258 const conf_path = try std.fs.path.join(arena, &.{ vendor, "mux.fish" }); 283 const conf_path = try std.fs.path.join(arena, &.{ vendor, "mux.fish" });
259 try writeFilePrivate(conf_path, fish_conf); 284 try writeFilePrivate(conf_path, fish_conf);
260 const orig = std.posix.getenv("XDG_DATA_DIRS") orelse "/usr/local/share:/usr/share"; 285 const orig = std.posix.getenv("XDG_DATA_DIRS") orelse "/usr/local/share:/usr/share";
@@ -439,17 +464,31 @@ test "install names the shim directory after the daemon and degrades in place" {
439 defer t.deinit(); 464 defer t.deinit();
440 465
441 // The naming rule, asserted where it now lives: the caller hands over a 466 // The naming rule, asserted where it now lives: the caller hands over a
442 // parent and gets back a per-pid directory under it, which is what keeps 467 // parent and gets back a directory under it named for this daemon, which
443 // two daemons sharing one runtime directory out of each other's shims. 468 // is what keeps two daemons sharing one runtime directory out of each
469 // other's shims.
444 const inj = install(arena.allocator(), t.dir, "/bin/bash"); 470 const inj = install(arena.allocator(), t.dir, "/bin/bash");
445 var want: [512]u8 = undefined; 471 var want: [512]u8 = undefined;
446 const expect = try std.fmt.bufPrint( 472 const prefix = try std.fmt.bufPrint(
447 &want, 473 &want,
448 "{s}/mux-shellint-{d}", 474 "{s}/mux-shellint-{d}-",
449 .{ t.dir, std.os.linux.getpid() }, 475 .{ t.dir, std.os.linux.getpid() },
450 ); 476 );
451 try std.testing.expectEqualStrings(expect, inj.dir.?); 477 // A prefix, not the whole name: the pid is followed by 12 hex digits of
452 try std.fs.cwd().access(expect, .{}); 478 // randomness, and the two halves answer different questions — the pid
479 // says which daemon a directory belongs to, the random part is what an
480 // attacker who can create entries in the parent cannot guess.
481 try std.testing.expect(std.mem.startsWith(u8, inj.dir.?, prefix));
482 try std.testing.expectEqual(prefix.len + 12, inj.dir.?.len);
483 try std.fs.cwd().access(inj.dir.?, .{});
484
485 // Twice in ONE process, so the pid is identical and only the random half
486 // can differ. This is the pinned oddity going away: a second daemon that
487 // drew a predecessor's pid used to find the leftover directory sitting
488 // there and lose its marks to it.
489 const again = install(arena.allocator(), t.dir, "/bin/bash");
490 try std.testing.expect(std.mem.startsWith(u8, again.dir.?, prefix));
491 try std.testing.expect(!std.mem.eql(u8, inj.dir.?, again.dir.?));
453 492
454 // An unwritable parent is the degraded path, and it is NOT an error: a 493 // An unwritable parent is the degraded path, and it is NOT an error: a
455 // session without marks still runs, so `install` returns the empty 494 // session without marks still runs, so `install` returns the empty
@@ -470,23 +509,77 @@ test "prepare zsh: a failure after the directory exists leaves no orphan" {
470 var t = try TmpPath.make(); 509 var t = try TmpPath.make();
471 defer t.deinit(); 510 defer t.deinit();
472 511
473 // Force the write-the-rc step to fail deterministically: put a 512 // The failure has to land AFTER the directory exists, and the shim root
474 // DIRECTORY where the .zshrc file needs to go, so writeFilePrivate's 513 // is now created exclusively — so nothing can be planted inside it in
475 // createFile fails with IsDir. makePrivateDir's makePath is idempotent, 514 // advance, and planting the root itself would fail the step before the
476 // so pre-creating the shim root here changes nothing about that step — 515 // one under test. An allocator that refuses its first request is what is
477 // the failure still lands after directory creation, same as a write 516 // left, and it fails exactly where the old planted `.zshrc` did: at the
478 // failing on a freshly created one. 517 // first step after the mkdir.
479 const shim = try std.fs.path.join(arena.allocator(), &.{ t.dir, "shim" }); 518 const shim = try std.fs.path.join(arena.allocator(), &.{ t.dir, "shim" });
480 const rc_path = try std.fs.path.join(arena.allocator(), &.{ shim, ".zshrc" }); 519 var failing = std.testing.FailingAllocator.init(arena.allocator(), .{ .fail_index = 0 });
481 try std.fs.cwd().makePath(rc_path);
482 520
483 try std.testing.expectError(error.IsDir, prepare(arena.allocator(), shim, "/usr/bin/zsh")); 521 try std.testing.expectError(
522 error.OutOfMemory,
523 prepare(failing.allocator(), shim, "/usr/bin/zsh"),
524 );
484 // The errdefer takes the whole directory with it rather than leaving 525 // The errdefer takes the whole directory with it rather than leaving
485 // an orphan for teardown to never hear about (Injection.dir is the 526 // an orphan for teardown to never hear about (Injection.dir is the
486 // only thing teardown deletes, and a failed prepare never returns one). 527 // only thing teardown deletes, and a failed prepare never returns one).
487 try std.testing.expectError(error.FileNotFound, std.fs.cwd().access(shim, .{})); 528 try std.testing.expectError(error.FileNotFound, std.fs.cwd().access(shim, .{}));
488 } 529 }
489 530
531 test "prepare refuses a shim path it did not create, and writes nothing through it" {
532 var arena = std.heap.ArenaAllocator.init(std.testing.allocator);
533 defer arena.deinit();
534 var t = try TmpPath.make();
535 defer t.deinit();
536
537 // The attack, in three lines. `parent_dir` is the socket's directory,
538 // which is a shared /tmp on any box with no XDG_RUNTIME_DIR, so another
539 // user can create an entry at the name this daemon is about to pick —
540 // and a symlink is the version that costs the victim something: the old
541 // makePath tolerated it, the chmod that followed re-moded the TARGET to
542 // 0700, and the .zshrc landed inside a directory this daemon does not
543 // own, to be sourced by a shell running as its user.
544 const victim = try std.fs.path.join(arena.allocator(), &.{ t.dir, "victim" });
545 try std.fs.cwd().makePath(victim);
546 var vd = try std.fs.cwd().openDir(victim, .{ .iterate = true });
547 defer vd.close();
548 try vd.chmod(0o755);
549 const shim = try std.fs.path.join(arena.allocator(), &.{ t.dir, "shim" });
550 try std.posix.symlink(victim, shim);
551
552 // mkdir neither follows the link nor adopts what is there.
553 try std.testing.expectError(error.DirExists, prepare(arena.allocator(), shim, "/usr/bin/zsh"));
554
555 // Nothing was written through the link, and the target's mode is the
556 // one its owner chose — the two halves of the clobber, asserted apart
557 // because a fix that only stopped one of them would look like a fix.
558 try std.testing.expectError(
559 error.FileNotFound,
560 vd.access(".zshrc", .{}),
561 );
562 const st = try vd.stat();
563 try std.testing.expectEqual(@as(u32, 0o755), @as(u32, @intCast(st.mode & 0o777)));
564
565 // ...and the link itself survives: a refusal that deleted what it found
566 // would be the same trespass by another name. (The errdefer that removes
567 // a half-built shim is armed AFTER the create, which is what makes this
568 // hold.)
569 var link_buf: [std.fs.max_path_bytes]u8 = undefined;
570 try std.testing.expectEqualStrings(victim, try std.fs.cwd().readLink(shim, &link_buf));
571
572 // A plain pre-existing directory is refused on the same grounds: `dir`
573 // is this daemon's to create or to do without, never to adopt.
574 const taken = try std.fs.path.join(arena.allocator(), &.{ t.dir, "taken" });
575 try std.fs.cwd().makePath(taken);
576 try std.testing.expectError(error.DirExists, prepare(arena.allocator(), taken, "/bin/bash"));
577 try std.testing.expectError(error.FileNotFound, std.fs.cwd().access(
578 try std.fs.path.join(arena.allocator(), &.{ taken, "bash-init.sh" }),
579 .{},
580 ));
581 }
582
490 test "prepare zsh: the shim directory is 0700 and the rc file 0600" { 583 test "prepare zsh: the shim directory is 0700 and the rc file 0600" {
491 var arena = std.heap.ArenaAllocator.init(std.testing.allocator); 584 var arena = std.heap.ArenaAllocator.init(std.testing.allocator);
492 defer arena.deinit(); 585 defer arena.deinit();
src/xdg.zig
Old New
@@ -106,10 +106,16 @@ pub fn hostCachePathFrom(
106 } 106 }
107 107
108 /// Create `dir` and everything above it, then tighten `dir` itself to 108 /// Create `dir` and everything above it, then tighten `dir` itself to
109 /// 0700. Every directory this project creates to hold something private — 109 /// 0700. The directories this project creates UNDER THE USER'S OWN HOME —
110 /// the key file's parent, the handoff cache's, and the shell-integration 110 /// the key file's parent and the handoff cache's — want exactly this, so
111 /// shim directory — wants exactly this, so the policy and the two subtle 111 /// the policy and the two subtle parts of it live here rather than in a
112 /// parts of it live here rather than in a copy per caller. 112 /// copy per caller.
113 ///
114 /// Adopts a directory already at `dir`, and resolves the path through any
115 /// symlink on the way, both of which are right under `~` (an existing
116 /// `~/.config/mux` must not make `keygen` refuse, and a user who symlinked
117 /// their config elsewhere meant it) and wrong anywhere a stranger can
118 /// create entries. Those callers want `makeNewPrivateDir`.
113 pub fn makePrivateDir(dir: []const u8) !void { 119 pub fn makePrivateDir(dir: []const u8) !void {
114 try std.fs.cwd().makePath(dir); 120 try std.fs.cwd().makePath(dir);
115 // makePath leaves 0755, which does not expose a contained file's 121 // makePath leaves 0755, which does not expose a contained file's
@@ -126,6 +132,36 @@ pub fn makePrivateDir(dir: []const u8) !void {
126 try d.chmod(0o700); 132 try d.chmod(0o700);
127 } 133 }
128 134
135 /// The same 0700 policy, for a directory in a parent this process does NOT
136 /// own — `$XDG_RUNTIME_DIR` or, when that is unset, a shared `/tmp`. The
137 /// difference from `makePrivateDir` is the whole point: this refuses an
138 /// entry that is already there instead of adopting it.
139 ///
140 /// `makePrivateDir` tolerates a pre-existing entry and reaches it through
141 /// whatever the path resolves to, which is correct under `~` and is a
142 /// symlink attack anywhere else: an entry pre-created by another user as a
143 /// symlink would have `makePath` succeed, the chmod land on the link's
144 /// TARGET, and the caller's files then be written inside a directory it
145 /// does not own. So: one `mkdir`, which never follows a symlink and never
146 /// adopts an existing entry, and a `no_follow` open for the mode.
147 ///
148 /// The mode is passed to `mkdir` rather than chmod'd on afterwards so the
149 /// directory is never briefly 0755; the chmod that follows is for the
150 /// umask, which can only take bits away and on an odd one (0500) would
151 /// leave a directory this daemon cannot itself use.
152 pub fn makeNewPrivateDir(dir: []const u8) !void {
153 std.posix.mkdir(dir, 0o700) catch |err| switch (err) {
154 // Not ours. Named separately from the other errors because it is
155 // the only one a caller can act on: whatever is there belongs to
156 // somebody else, and the answer is to do without, never to use it.
157 error.PathAlreadyExists => return error.DirExists,
158 else => |e| return e,
159 };
160 var d = try std.fs.cwd().openDir(dir, .{ .iterate = true, .no_follow = true });
161 defer d.close();
162 try d.chmod(0o700);
163 }
164
129 /// The same, for callers holding the path of the FILE that is going to 165 /// The same, for callers holding the path of the FILE that is going to
130 /// live there. A `path` with no directory component is a no-op. 166 /// live there. A `path` with no directory component is a no-op.
131 pub fn makePrivateParent(path: []const u8) !void { 167 pub fn makePrivateParent(path: []const u8) !void {
@@ -224,6 +260,46 @@ test "makePrivateParent: a path with no directory part is a no-op" {
224 try makePrivateParent("bare-name-no-dir"); 260 try makePrivateParent("bare-name-no-dir");
225 } 261 }
226 262
263 test "makeNewPrivateDir: creates 0700, and refuses anything already there" {
264 const testtmp = @import("testtmp");
265 var tmp = try testtmp.TmpDir.make();
266 defer tmp.cleanup();
267
268 var buf: [128]u8 = undefined;
269 const dir = try std.fmt.bufPrint(&buf, "{s}/shim", .{tmp.path()});
270 try makeNewPrivateDir(dir);
271
272 var d = try std.fs.cwd().openDir(dir, .{ .iterate = true });
273 defer d.close();
274 const st = try d.stat();
275 try std.testing.expectEqual(@as(u32, 0o700), @as(u32, @intCast(st.mode & 0o777)));
276
277 // The whole difference from makePrivateDir: a second call does not
278 // succeed by adopting the first call's directory. A caller that treated
279 // DirExists as "fine, it exists" would have re-opened the hole this
280 // function exists to close.
281 try std.testing.expectError(error.DirExists, makeNewPrivateDir(dir));
282
283 // Any kind of entry, not just a directory: a regular file and a symlink
284 // are the two an attacker plants, and both must read as taken.
285 var fbuf: [128]u8 = undefined;
286 const file = try std.fmt.bufPrint(&fbuf, "{s}/plain", .{tmp.path()});
287 try std.fs.cwd().writeFile(.{ .sub_path = file, .data = "x" });
288 try std.testing.expectError(error.DirExists, makeNewPrivateDir(file));
289
290 var lbuf: [128]u8 = undefined;
291 const link = try std.fmt.bufPrint(&lbuf, "{s}/link", .{tmp.path()});
292 try std.posix.symlink(dir, link);
293 try std.testing.expectError(error.DirExists, makeNewPrivateDir(link));
294
295 // Parents are NOT created, which is the other half of "one mkdir": a
296 // caller whose parent is missing hears about it rather than having a
297 // tree built for it under a directory it does not control.
298 var nbuf: [128]u8 = undefined;
299 const nested = try std.fmt.bufPrint(&nbuf, "{s}/missing/leaf", .{tmp.path()});
300 try std.testing.expectError(error.FileNotFound, makeNewPrivateDir(nested));
301 }
302
227 test "writeNewKey: creates 0600 with 32 bytes, refuses to overwrite" { 303 test "writeNewKey: creates 0600 with 32 bytes, refuses to overwrite" {
228 const testtmp = @import("testtmp"); 304 const testtmp = @import("testtmp");
229 var tmp = try testtmp.TmpDir.make(); 305 var tmp = try testtmp.TmpDir.make();