From 3c24218d8567253d71f3827f937e23bd80cee9ac Mon Sep 17 00:00:00 2001 From: Jiri Cincura Date: Tue, 21 Apr 2026 12:12:12 +0200 Subject: [PATCH] Harden permissions for pipe used in SharedServer. --- .../IO/Pipes/NamedPipeServerStream.Unix.cs | 31 +++++++++-- .../NamedPipeTest.CurrentUserOnly.Unix.cs | 53 +++++++++++++++++++ 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Unix.cs b/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Unix.cs index 7a0b6dc8e37055..e07b6f274d6625 100644 --- a/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Unix.cs +++ b/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Unix.cs @@ -244,6 +244,8 @@ private sealed class SharedServer private readonly int _maxCount; /// The concurrent number of concurrent streams using this instance. private int _currentCount; + /// Whether the socket file mode has been tightened to current-user-only (0600). + private bool _isCurrentUserOnly; internal static SharedServer Get(string path, int maxCount, PipeOptions pipeOptions) { @@ -252,9 +254,9 @@ internal static SharedServer Get(string path, int maxCount, PipeOptions pipeOpti lock (s_servers) { - SharedServer? server; bool isFirstPipeInstance = (pipeOptions & PipeOptions.FirstPipeInstance) != 0; - if (s_servers.TryGetValue(path, out server)) + bool isCurrentUserOnly = (pipeOptions & PipeOptions.CurrentUserOnly) != 0; + if (s_servers.TryGetValue(path, out SharedServer? server)) { // On Windows, if a subsequent server stream is created for the same pipe and with a different // max count, the subsequent count is largely ignored in that it doesn't change the number of @@ -269,11 +271,21 @@ internal static SharedServer Get(string path, int maxCount, PipeOptions pipeOpti { throw new UnauthorizedAccessException(SR.Format(SR.UnauthorizedAccess_IODenied_Path, path)); } + + // Ratchet to current-user-only if requested. We never loosen, even if a later + // instance for the same path does not request CurrentUserOnly: silently downgrading + // a security choice another caller made would be worse than the order-dependent + // surprise of a non-CurrentUserOnly server inheriting a 0600 socket. + if (isCurrentUserOnly && !server._isCurrentUserOnly) + { + File.SetUnixFileMode(path, UnixFileMode.UserRead | UnixFileMode.UserWrite); + server._isCurrentUserOnly = true; + } } else { // No instance exists yet for this path. Create one a new. - server = new SharedServer(path, maxCount, isFirstPipeInstance); + server = new SharedServer(path, maxCount, isFirstPipeInstance, isCurrentUserOnly); s_servers.Add(path, server); } @@ -308,7 +320,7 @@ internal void Dispose(bool disposing) } } - private SharedServer(string path, int maxCount, bool isFirstPipeInstance) + private SharedServer(string path, int maxCount, bool isFirstPipeInstance, bool isCurrentUserOnly) { if (!isFirstPipeInstance) { @@ -325,6 +337,13 @@ private SharedServer(string path, int maxCount, bool isFirstPipeInstance) { socket.Bind(new UnixDomainSocketEndPoint(path)); isSocketBound = true; + + _isCurrentUserOnly = isCurrentUserOnly; + if (_isCurrentUserOnly) + { + File.SetUnixFileMode(path, UnixFileMode.UserRead | UnixFileMode.UserWrite); + } + socket.Listen(int.MaxValue); } catch (SocketException) when (isFirstPipeInstance && !isSocketBound) @@ -334,6 +353,10 @@ private SharedServer(string path, int maxCount, bool isFirstPipeInstance) } catch { + if (isSocketBound) + { + Interop.Sys.Unlink(path); // ignore any failures + } socket.Dispose(); throw; } diff --git a/src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.Unix.cs b/src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.Unix.cs index 2ff8d6436e9601..2e2ec6d6d35079 100644 --- a/src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.Unix.cs +++ b/src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.Unix.cs @@ -69,6 +69,59 @@ public async Task Connection_UnderDifferentUsers_BehavesAsExpected( } } + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/77469", TestPlatforms.iOS | TestPlatforms.tvOS)] + [ActiveIssue("https://github.com/dotnet/runtime/issues/77470", TestPlatforms.LinuxBionic)] + public void CurrentUserOnly_SetsUnixFileMode() + { + string pipeName = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + + using (var server = new NamedPipeServerStream(pipeName, PipeDirection.InOut, 1, PipeTransmissionMode.Byte, PipeOptions.CurrentUserOnly)) + { + Assert.Equal(UnixFileMode.UserRead | UnixFileMode.UserWrite, File.GetUnixFileMode(pipeName)); + } + } + + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/77469", TestPlatforms.iOS | TestPlatforms.tvOS)] + [ActiveIssue("https://github.com/dotnet/runtime/issues/77470", TestPlatforms.LinuxBionic)] + public void CurrentUserOnly_SubsequentInstance_TightensSharedMode() + { + string pipeName = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + + using (var first = new NamedPipeServerStream(pipeName, PipeDirection.InOut, 2, PipeTransmissionMode.Byte, PipeOptions.None)) + { + UnixFileMode initialMode = File.GetUnixFileMode(pipeName); + Assert.NotEqual(UnixFileMode.UserRead | UnixFileMode.UserWrite, initialMode); + + using (var second = new NamedPipeServerStream(pipeName, PipeDirection.InOut, 2, PipeTransmissionMode.Byte, PipeOptions.CurrentUserOnly)) + { + Assert.Equal(UnixFileMode.UserRead | UnixFileMode.UserWrite, File.GetUnixFileMode(pipeName)); + } + + // Mode is one-way: it stays restrictive even after the CurrentUserOnly instance is disposed. + Assert.Equal(UnixFileMode.UserRead | UnixFileMode.UserWrite, File.GetUnixFileMode(pipeName)); + } + } + + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/77469", TestPlatforms.iOS | TestPlatforms.tvOS)] + [ActiveIssue("https://github.com/dotnet/runtime/issues/77470", TestPlatforms.LinuxBionic)] + public void CurrentUserOnly_SubsequentInstance_DoesNotLoosenSharedMode() + { + string pipeName = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + + using (var first = new NamedPipeServerStream(pipeName, PipeDirection.InOut, 2, PipeTransmissionMode.Byte, PipeOptions.CurrentUserOnly)) + { + Assert.Equal(UnixFileMode.UserRead | UnixFileMode.UserWrite, File.GetUnixFileMode(pipeName)); + + using (var second = new NamedPipeServerStream(pipeName, PipeDirection.InOut, 2, PipeTransmissionMode.Byte, PipeOptions.None)) + { + Assert.Equal(UnixFileMode.UserRead | UnixFileMode.UserWrite, File.GetUnixFileMode(pipeName)); + } + } + } + private static void ConnectClientFromRemoteInvoker(string pipeName, string isCurrentUserOnly, string isReadOnly) { PipeOptions pipeOptions = bool.Parse(isCurrentUserOnly) ? PipeOptions.CurrentUserOnly : PipeOptions.None;