dotnet / dotnet/dev-proxy

State/liveness keyed on PID alone is vulnerable to PID reuse

Open
#1,755 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C#
Stars
832
Forks
89
Avg merge
12h 1m
Merged PRs (30d)
23

Description

## Description

Dev Proxy tracks detached instances by PID alone (`state-.json`), and `StateManager` determines whether an instance is "alive" purely by checking whether a process with that PID currently exists (`StateManager.IsProcessRunning(int pid)` → `Process.GetProcessById`).

Operating systems recycle PIDs. After an instance exits (cleanly or via crash), its PID number can be reassigned to a completely unrelated process. When that happens:

- `IsProcessRunning(pid)` returns `true` for the stale state record, so Dev Proxy believes its old instance is still alive.
- Commands that key on liveness (`stop`, `status`, `LoadAllStatesAsync`, `FindSystemProxyInstanceAsync`, and the crash-recovery / orphaned-system-proxy reconciliation added in the `stop --force` fix) can act on — or refuse to act on — the wrong process.

This is a pre-existing, low-probability correctness issue in the whole detached-instance design; it is independent of any single command.

## Suggested fix

Store additional identity beyond the PID in the state file and verify it before treating a PID as "our" live instance. Options:

- Persist the process **start time** (`Process.StartTime`) alongside the PID, and in `IsProcessRunning` compare the running process's start time to the recorded value — a mismatch means the PID was reused and the record is stale.
- Optionally also persist the process name / a Dev Proxy marker as a secondary check.

`Process.StartTime` is available cross-platform in .NET and is the standard, low-cost way to disambiguate PID reuse.

## Notes

- Found while implementing the fix for #1731 (`devproxy stop --force` cannot restore the system proxy after a crashed instance). That fix relies on `asSystemProxy` state records to reconcile orphaned system-proxy registrations; PID-reuse hardening would make that reconciliation (and all liveness checks) more robust but is intentionally out of scope for that change.
- Affected code: `DevProxy/State/StateManager.cs` (`IsProcessRunning`, `LoadStateFromFileAsync`, `GetOrphanedSystemProxyStatesAsync`), `DevProxy/State/ProxyInstanceState.cs`.

Contributor guide

Open the contributing guide

Research direction

Start by reading DevProxy/State/StateManager.cs, especially IsProcessRunning, LoadStateFromFileAsync, and GetOrphanedSystemProxyStatesAsync, then inspect DevProxy/State/ProxyInstanceState.cs to understand the state-file data. Trace how detached-instance commands use PID liveness. Done means stale state is rejected when a PID has been reused, while legitimate instances continue to be recognized.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
cli, devtools
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.