authorgravatar for egoist@egoistic.devxEgoist <egoist@egoistic.dev> 2023-05-04 19:42:27-05:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2024-01-16 01:25:14-08:00
log194ed308259e49af3f4725659e22ccd7457404e0
treeda51480a82b6158902496d032115990001520e8e
parent41bf816fa6dd6fde0264db885c2f00757aba74b0

child_process: Use security attributes while creating handle.

As suggested by @matu3ba, it can be better to use Security Attributes directly while creating the handle instead of creating the handle then setting the handle to inherit. Doing so can prevent potentially leaking to other parallel spawned processes which would inherit the opened `\Device\Null` handle. This change also allows windows.OpenFile to handle when bInheritHandle is set. Note that we are using the same `saAttr`, but since it's taken as a pointer to a const in all calls, it's never mutated, and OpenFile never alters it. This also saves 1 kernel call for setting the handle to inherit.

2 files changed, 6 insertions(+), 5 deletions(-)

lib/std/child_process.zig+2-4
...@@ -650,7 +650,7 @@ pub const ChildProcess = struct {...@@ -650,7 +650,7 @@ pub const ChildProcess = struct {
650 }650 }
651651
652 fn spawnWindows(self: *ChildProcess) SpawnError!void {652 fn spawnWindows(self: *ChildProcess) SpawnError!void {
653 const saAttr = windows.SECURITY_ATTRIBUTES{653 var saAttr = windows.SECURITY_ATTRIBUTES{
654 .nLength = @sizeOf(windows.SECURITY_ATTRIBUTES),654 .nLength = @sizeOf(windows.SECURITY_ATTRIBUTES),
655 .bInheritHandle = windows.TRUE,655 .bInheritHandle = windows.TRUE,
656 .lpSecurityDescriptor = null,656 .lpSecurityDescriptor = null,
...@@ -663,6 +663,7 @@ pub const ChildProcess = struct {...@@ -663,6 +663,7 @@ pub const ChildProcess = struct {
663 windows.OpenFile(&[_]u16{ '\\', 'D', 'e', 'v', 'i', 'c', 'e', '\\', 'N', 'u', 'l', 'l' }, .{663 windows.OpenFile(&[_]u16{ '\\', 'D', 'e', 'v', 'i', 'c', 'e', '\\', 'N', 'u', 'l', 'l' }, .{
664 .access_mask = windows.GENERIC_READ | windows.GENERIC_WRITE | windows.SYNCHRONIZE,664 .access_mask = windows.GENERIC_READ | windows.GENERIC_WRITE | windows.SYNCHRONIZE,
665 .share_access = windows.FILE_SHARE_READ | windows.FILE_SHARE_WRITE,665 .share_access = windows.FILE_SHARE_READ | windows.FILE_SHARE_WRITE,
666 .sa = &saAttr,
666 .creation = windows.OPEN_EXISTING,667 .creation = windows.OPEN_EXISTING,
667 .io_mode = .blocking,668 .io_mode = .blocking,
668 }) catch |err| switch (err) {669 }) catch |err| switch (err) {
...@@ -680,9 +681,6 @@ pub const ChildProcess = struct {...@@ -680,9 +681,6 @@ pub const ChildProcess = struct {
680 defer {681 defer {
681 if (any_ignore) os.close(nul_handle);682 if (any_ignore) os.close(nul_handle);
682 }683 }
683 if (any_ignore) {
684 try windows.SetHandleInformation(nul_handle, windows.HANDLE_FLAG_INHERIT, windows.HANDLE_FLAG_INHERIT);
685 }
686684
687 var g_hChildStd_IN_Rd: ?windows.HANDLE = null;685 var g_hChildStd_IN_Rd: ?windows.HANDLE = null;
688 var g_hChildStd_IN_Wr: ?windows.HANDLE = null;686 var g_hChildStd_IN_Wr: ?windows.HANDLE = null;
lib/std/os/windows.zig+4-1
...@@ -86,7 +86,10 @@ pub fn OpenFile(sub_path_w: []const u16, options: OpenFileOptions) OpenError!HAN...@@ -86,7 +86,10 @@ pub fn OpenFile(sub_path_w: []const u16, options: OpenFileOptions) OpenError!HAN
86 var attr = OBJECT_ATTRIBUTES{86 var attr = OBJECT_ATTRIBUTES{
87 .Length = @sizeOf(OBJECT_ATTRIBUTES),87 .Length = @sizeOf(OBJECT_ATTRIBUTES),
88 .RootDirectory = if (std.fs.path.isAbsoluteWindowsWTF16(sub_path_w)) null else options.dir,88 .RootDirectory = if (std.fs.path.isAbsoluteWindowsWTF16(sub_path_w)) null else options.dir,
89 .Attributes = 0, // Note we do not use OBJ_CASE_INSENSITIVE here.89 .Attributes = if (options.sa) |ptr| blk: { // Note we do not use OBJ_CASE_INSENSITIVE here.
90 const inherit: ULONG = if (ptr.bInheritHandle == TRUE) OBJ_INHERIT else 0;
91 break :blk inherit;
92 } else 0,
90 .ObjectName = &nt_name,93 .ObjectName = &nt_name,
91 .SecurityDescriptor = if (options.sa) |ptr| ptr.lpSecurityDescriptor else null,94 .SecurityDescriptor = if (options.sa) |ptr| ptr.lpSecurityDescriptor else null,
92 .SecurityQualityOfService = null,95 .SecurityQualityOfService = null,