[SignalR] Client-result invocation IDs are a predictable process-wide counter, and the return-type lookup is not connection-scoped
- Dominant language
- C#
- Stars
- 38.4k
- Forks
- 10.9k
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 281
Description
**Observed on:** `release/10.0` @ 10.0.13, commit `1307f36b2990bf5c81aace7c6a1535a2be46d7ef`. I have not checked the 8.0/9.0 branches.
### Summary
Two defence-in-depth gaps in the client-results path. Neither is exploitable today
because the ownership check in `TryCompleteResult` holds, but both weaken it
unnecessarily.
### 1. Invocation IDs are predictable across all connections
`src/SignalR/server/Core/src/DefaultHubLifetimeManager.cs:345`
```csharp
var id = Interlocked.Increment(ref _lastInvocationId);
var invocationId = $"s{id}";
```
`_lastInvocationId` is a single `ulong` field on the lifetime manager, shared by
every connection on the server, so IDs are `s1`, `s2`, `s3`... Any client can
enumerate the invocation IDs issued to every other client.
### 2. The return-type lookup is not connection-scoped
`src/SignalR/common/Shared/ClientResultsManager.cs:81,91` — `TryGetType` and
`GetReturnType` resolve by invocation ID alone, with no connection argument, and
run during message parsing, i.e. before any ownership check.
By contrast `TryCompleteResult` (line 52) *does* check ownership:
```csharp
if (item.ConnectionId != connectionId)
{
throw new InvalidOperationException($"Connection ID '{connectionId}' is not valid for invocation ID '{message.InvocationId}'.");
}
```
### Residual behaviour
Because the ownership check holds, a client cannot supply the result for another
client's invocation. What remains is an oracle: sending a `CompletionMessage` for a
guessed `sN` produces two distinguishable outcomes at
`DefaultHubDispatcher.cs:213` —
- `sN` pending for any connection -> `SetConnectionResultAsync` -> ownership throw
-> the probing connection is torn down.
- `sN` not pending -> `Log.UnexpectedCompletion` -> the probing connection survives.
So a client can test whether a given invocation ID is outstanding server-wide, at a
cost of one connection per probe. This discloses invocation-counter activity, not
user data.
### Suggested fix
1. Make invocation IDs unguessable, or scope the counter per connection so one
client's IDs say nothing about another's.
2. Give `TryGetReturnType` / `TryGetType` a `connectionId` parameter so the type
lookup is scoped the same way completion already is. This removes the oracle and
means the parse path no longer deserialises a payload against a type belonging
to a different connection's pending invocation.
### Verification status
Source-level trace, not an executed repro — building this servicing branch needs the
runtime toolset `10.0.13-servicing.26454.107`, which is not on the public feed. Noting
that rather than implying I observed the oracle behaviour at runtime. The ownership
check in `TryCompleteResult` is unambiguous in source, so I am confident this is a
hardening item and **not** a hijacking bug; please read it that way.
Contributor guide
Research direction
Start with DefaultHubLifetimeManager.cs around line 345 and ClientResultsManager.cs around lines 52, 81, and 91, then trace parsing through DefaultHubDispatcher.cs around line 213. Compare the connection-scoped ownership check with the current invocation-ID and return-type lookups. Done means IDs no longer expose cross-connection activity and return-type resolution is scoped consistently; runtime verification requires the unavailable 10.0.13-servicing.26454.107 toolset.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- backend, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100