Skip to content

fix(callbacks): bind RPC responses to their source and use unpredictable ids - #185

Open
Pajt9whauht283as wants to merge 1 commit into
VORPCORE:mainfrom
Pajt9whauht283as:fix/rpc-response-source-binding
Open

fix(callbacks): bind RPC responses to their source and use unpredictable ids#185
Pajt9whauht283as wants to merge 1 commit into
VORPCORE:mainfrom
Pajt9whauht283as:fix/rpc-response-source-binding

Conversation

@Pajt9whauht283as

Copy link
Copy Markdown

Summary

As discussed in #182 — pending client-callback responses were stored keyed only by a sequential, shared counter (name .. N), and the response handler never verified that the responding player was the one the request was sent to. Any connected client could resolve another player's pending callback with forged data by guessing ids.

Changes

  • Pending calls are now keyed with random per-call tokens instead of the shared sequential callBackId
  • Each pending entry records the initiating source
  • ExecuteRpc rejects responses whose source does not match the initiator
  • Async/sync dispatch now strictly checks what is actually pending (promise vs callback), avoiding type-confusion if a client tampers with isSync on its own response

No API changes — TriggerAsync / TriggerAwait / Register signatures and behavior are unchanged for legitimate flows.

Fixes #182

…ble ids

Pending client-callback responses were stored in a table keyed only by a
sequential, shared counter (name .. N), and the response handler never
verified that the responding player was the one the request was sent to.
Any connected client could therefore resolve another player's pending
callback with forged data by guessing ids.

- key pending calls with random per-call tokens
- record the initiating source alongside each pending callback/promise
- reject responses whose source does not match the initiator
- dispatch async/sync strictly by what is actually pending

Fixes VORPCORE#182
@Pajt9whauht283as

Copy link
Copy Markdown
Author

Note for reviewers: no API changes \u2014 \TriggerAsync/\TriggerAwait/\Register\ signatures and behavior are identical for legitimate flows; only id generation, source-binding and dispatch safety changed.

@dop42

dop42 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Before outsider review it and potentially merge it, could you put the PR in the repo's template format please, and take a pass on a few review points: cleanup for orphaned TriggeredCalls entries (not sure if this is already handled elsewhere in the code, but wanted to flag it), the duplication between TriggerRpcAsync and TriggerRpcAwait, and the promise variable shadowing the module name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security finding - ServerCallback responses never bound to source; shared sequential IDs make forgery trivial

2 participants