Skip to content

execute_subscription permits unbounded rapid catch-up execution after keeper downtime #24

Description

@abayomicornelius

Problem

create_payment_request (stellar_send/src/payment_request.rs lines 52-89) validates amount > 0, expiry > now, and payer != requester (when payer is Some) — but does not validate token at all (no zero-address-style check, though Soroban Address doesn't have a universal "zero" sentinel the way EVM does, so this would need to be a plausibility check like confirming the address corresponds to a deployed contract, which Soroban doesn't easily support pre-invocation either) and, more concretely, does not enforce any minimum window between now and expiryexpiry = now + 1 (one second in the future) passes validation.

Why it matters

An expiry just barely in the future is a realistic griefing vector against the requester's own intent: if a merchant creates an invoice expecting a customer to have a reasonable window to pay (e.g. hours or days), but a UI bug or malicious integration passes a near-immediate expiry, the request becomes effectively unfulfillable moments after creation, and there's no contract-level floor preventing this class of mistake — fulfill_payment_request (lines 94-141) simply reports RequestExpired after the fact rather than the system having guarded against an obviously-too-short window at creation time.

Proposed fix

Add a MIN_EXPIRY_WINDOW_SECONDS: u64 constant (e.g. 60 seconds, generous enough to never block legitimate immediate-use cases while catching obvious mistakes) and check expiry < env.ledger().timestamp().checked_add(MIN_EXPIRY_WINDOW_SECONDS)... returning StellarSendError::InvalidExpiry (already exists, variant 21) for windows shorter than that floor.

Edge cases

  • Must use checked_add for the now + MIN_EXPIRY_WINDOW_SECONDS comparison to avoid a u64 overflow panic on pathological now values (unlikely in practice given realistic ledger timestamps, but consistent with this codebase's general discipline of guarding arithmetic).
  • Should also consider a maximum expiry window (e.g. reject expiry more than a year out) to bound how long stale, forgotten payment requests linger in persistent storage — a smaller, related enhancement worth bundling into the same PR since it uses the same validation pattern.

Testing strategy

Add test_create_payment_request_rejects_too_short_expiry asserting Err(StellarSendError::InvalidExpiry) for expiry = now + 1, and test_create_payment_request_accepts_minimum_valid_window as a boundary test at exactly MIN_EXPIRY_WINDOW_SECONDS.

Metadata

Metadata

Assignees

No one assigned

    Labels

    GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardOfficial Campaign | FWC26Campaign: Official Campaign | FWC26contractsSmart contract logicsecuritySecurity concernvery hardVery difficult / senior-level bounty issue

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions