dotnet / dotnet/runtime

NamedPipeServerStream on Unix silently deletes existing non-socket file for rooted pipe names

Open
#131,200 1 comment 0 reactions 0 assignees View on GitHub
area-System.IO needs-further-triage
Dominant language
C#
Stars
18.3k
Forks
5.6k
PR merge metrics
PR metrics pending

Description

### Description

The Unix `NamedPipeServerStream`'s constructor unlinks whatever already exists at the pipe's socket path, without checking that it is a socket. A rooted pipe name is used verbatim as the socket path, so constructing a server with a rooted name that points at an ordinary file silently deletes that file.

I ported a Windows-based library to Linux which creates a pipe server. To my very great surprise, the test-run deleted the application DLL. This is because the library defaults to `Environment.GetCommandLineArgs()[0]` as the socket name. Of course, on Windows a socket is just a kernel object, but on Unix it's a real file.

### Reproduction Steps

```csharp
using System.IO.Pipes;

Directory.CreateDirectory("/tmp/repro");
var path = "/tmp/repro/data.bin";
File.WriteAllText(path, "data");

Console.WriteLine($"exists before: {File.Exists(path)}");

using (new NamedPipeServerStream(path, PipeDirection.InOut, 1, PipeTransmissionMode.Byte, PipeOptions.Asynchronous))
{
Console.WriteLine("server constructed");
}

Console.WriteLine($"exists after: {File.Exists(path)}");
```

### Expected behavior

```
exists before: True
server constructed
```
...followed by an exception or other indication of failure

### Actual behavior

```
exists before: True
server constructed
exists after: False
```

### Regression?

Unknown.

### Known Workarounds

Provide a non-rooted name ... if you can figure out that's the problem.

### Configuration

* .NET SDK 10.0.301, runtime 10.0.9
* Debian Linux 6.12.74 (Devuan), x64
* Runtime source code from 10.0

### Other information

Given the comments in the `SharedServer` ctor in `NamedPipeServerStream.Unix.cs`:

```csharp
if (!isFirstPipeInstance)
{
// Binding to an existing path fails, so we need to remove anything left over at this location.
// There's of course a race condition here, where it could be recreated by someone else between this
// deletion and the bind below, in which case we'll simply let the bind fail and throw.
Interop.Sys.Unlink(path); // ignore any failures
}
```

The unlink is clearly intentional because a crashed server would orphan the socket file and any subsequent run would report the path in-use.

It seems to me the runtime should `stat` the path first and unlink only if the existing inode is actually a socket. If not, throw an exception (or whatever other failure mode is appropriate).

Possibly related: `GetPipePath` in `PipeStream.Unix.cs` returns a rooted pipe name verbatim, bypassing the `CoreFxPipe_` temp-dir prefixing used for non-rooted paths. I suppose the `stat` check could happen there, too.

Contributor guide

Open the contributing guide

Research direction

Start with NamedPipeServerStream.Unix.cs, especially the SharedServer constructor and its unlink call, then inspect GetPipePath in PipeStream.Unix.cs for rooted names. Run the provided reproduction on Unix. Done means constructing a server for a rooted path does not delete an existing ordinary file and instead reports an appropriate failure, while orphaned socket cleanup still works.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
operating-systems
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.