Skip to content

fix: eliminate push-mode semaphore exhaustion issue#44

Merged
marcelosalloum merged 4 commits intomainfrom
fix/push-mode-dos-semaphore-exhaustion
Apr 20, 2026
Merged

fix: eliminate push-mode semaphore exhaustion issue#44
marcelosalloum merged 4 commits intomainfrom
fix/push-mode-dos-semaphore-exhaustion

Conversation

@marcelosalloum
Copy link
Copy Markdown
Collaborator

@marcelosalloum marcelosalloum commented Apr 15, 2026

What

  • Replace the unbounded pollTransaction loop with a single getTransaction call for push-mode (hash) credentials — fake hashes are rejected instantly without acquiring a semaphore slot
  • Add schema-level regex validation (/^[0-9a-f]{64}$/i) to the hash field in Methods.ts, rejecting malformed hashes at the Zod layer before any server-side processing
  • Push mode now requires the transaction to be confirmed on-chain before the client submits the hash

Why

PR #42 replaced the global verifyLock with a Semaphore(10), but an attacker could still exhaust all semaphore slots by submitting format-valid fake hashes (e.g. deadbeef repeated) that each held a slot for the full pollTimeoutMs (20s). Ten requests every 20 seconds permanently blocks all push-mode verifications.

This change removes polling from push mode entirely. A single getTransaction call returns instantly for non-existent hashes (NOT_FOUND), eliminating the attack vector. Pull-mode (transaction) polling and its semaphore are unchanged — the server controls those hashes.

Copilot AI review requested due to automatic review settings April 15, 2026 22:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the Stellar charge server’s push-mode verification path against semaphore-exhaustion DoS by removing polling behavior for client-submitted transaction hashes and adding stricter hash validation at the schema layer.

Changes:

  • Replace push-mode pollTransaction usage with a single rpcServer.getTransaction() lookup and fail fast on NOT_FOUND/FAILED.
  • Add Zod schema regex validation for credential.payload.hash (/^[0-9a-f]{64}$/i).
  • Add/adjust tests to cover immediate rejection for NOT_FOUND/FAILED and concurrent fake-hash requests without semaphore contention.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
sdk/src/charge/server/Charge.ts Removes push-mode polling and adds single-lookup status handling for hash verification.
sdk/src/charge/server/Charge.test.ts Adds tests ensuring push-mode fast-failure and no semaphore contention.
sdk/src/charge/Methods.ts Tightens schema validation for push-mode hash credentials.
sdk/src/charge/Methods.test.ts Adds schema tests for valid/invalid hash formats.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread sdk/src/charge/server/Charge.ts Outdated
Comment thread sdk/src/charge/server/Charge.ts Outdated
@marcelosalloum marcelosalloum self-assigned this Apr 15, 2026
@github-project-automation github-project-automation Bot moved this to Backlog (Not Ready) in DevX Apr 15, 2026
@marcelosalloum marcelosalloum moved this from Backlog (Not Ready) to Needs Review in DevX Apr 15, 2026
@marcelosalloum marcelosalloum changed the title fix: eliminate push-mode DoS via semaphore exhaustion fix: eliminate push-mode semaphore exhaustion issue Apr 15, 2026
@marcelosalloum marcelosalloum merged commit cddd6d3 into main Apr 20, 2026
7 checks passed
@github-project-automation github-project-automation Bot moved this from Needs Review to Done in DevX Apr 20, 2026
@marcelosalloum marcelosalloum deleted the fix/push-mode-dos-semaphore-exhaustion branch April 20, 2026 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants