Skip to content

PayRequest page lets the payer silently edit the requested amount and destination before paying #26

Description

@abayomicornelius

Problem

src/pages/PayRequest.tsx renders the normal, fully-editable SendForm to let someone pay a payment request, merely pre-filling it from the fetched paymentRequest:

{(state.step === 'form' || state.step === 'error') && (
  <SendForm
    onSubmit={requestQuote}
    isLoading={isQuoting}
    supportedAssets={supportedAssets}
    defaultValues={{
      destinationAddress: paymentRequest.requesterPublicKey,
      amount: paymentRequest.amount,
      sourceAssetCode: paymentRequest.assetCode,
      destinationAssetCode: paymentRequest.assetCode,
      memo: paymentRequest.memo ?? '',
    }}
  />
)}

defaultValues on a react-hook-form-backed component (the same pattern used by BatchForm, EscrowForm, SubscriptionForm elsewhere in this codebase, all of which accept a defaultValues prop purely as an initial value, not a locked one) only seeds the form's initial state — nothing in SendForm, requestQuote, or confirmSend re-validates that the amount/destination the payer actually submits still matches paymentRequest.amount/paymentRequest.requesterPublicKey by the time confirmSend fires.

Why it matters

  • The entire premise of a "payment request" (as communicated by the UI itself — pages/PayRequest.tsx shows a large, prominent {formatAmount(paymentRequest.amount, 4)} figure as "the" amount, and ConfirmEscrowModal/ConfirmSubscriptionModal-style confirm screens elsewhere in this app exist specifically to give users one last chance to review before signing) is that the payer is fulfilling a specific request for a specific amount to a specific requester. Because the send form underneath is just the ordinary, fully-editable SendForm, a payer can change the amount and/or destination address before submitting, and the resulting quote/confirmation flow (QuoteCard, ConfirmModal) has no way to know or flag that what's about to be signed no longer matches the request it was opened from.
  • This matters most in exactly the scenario payment requests are built for — invoicing, where the requester and payer may not be the same trusted party, and any silent divergence between "what was requested" and "what was actually paid" undermines the feature's reason to exist (a payer could accidentally, or a malicious clipboard-hijacking extension could deliberately, alter the destination address in the prefilled field before signing, and nothing downstream would catch it, since the form treats it as an ordinary edited send).
  • There's also no re-validation against paymentRequest.status at submit time — the page checks paymentRequest.status !== 'open' once during render, but if the request expires or is cancelled by the requester in the seconds between the page loading and the payer clicking "confirm" (a real race given usePaymentRequest's staleTime: 15_000 — the local copy of paymentRequest can be up to 15 seconds stale), the payer isn't warned and can complete a payment against a request the requester believes is no longer valid.

Reproduction

  1. Open a /pay/:id link for an open payment request for, say, 50 XLM.
  2. Edit the amount field down to 5 XLM before submitting.
  3. Complete the send — it goes through as an ordinary 5 XLM payment, with the request's own record (and the requester, watching for the requested 50 XLM) none the wiser that a different amount was actually sent under the guise of "fulfilling" their request.

Suggested fix

  • Make the amount and destination fields read-only (not merely pre-filled) when SendForm is rendered from PayRequest — add a prop (e.g. lockedFields or a readOnly variant) to SendForm specifically for this call site, so the payer can only adjust things like the memo or, if relevant, path-payment slippage, but not the core amount/destination.
  • Before calling confirmSend, re-fetch (refetch()) the payment request and hard-block if its status is no longer 'open' or its amount/requesterPublicKey no longer match what was originally displayed, surfacing a clear "This request has changed — please reload" error instead of proceeding.

Edge cases

  • Some payment-request products deliberately allow the payer to pay a different amount (e.g. "pay what you want" tipping-style requests) — if that's ever a desired feature here, the fix should make locked-vs-editable an explicit, visible choice in the UI (e.g. "Requested amount: 50 XLM [Edit]") rather than the current silent, unannounced editability.

Testing strategy

  • Add a PayRequest.test.tsx (none currently exists) rendering the page with a mocked usePaymentRequest payload, asserting the amount/destination fields are disabled/read-only in the rendered SendForm.
  • Add a test simulating the payment request transitioning to 'expired'/'cancelled' between initial load and confirm, asserting the confirm action is blocked with a clear message rather than allowed to proceed.

Related issues in this batch

Shares the "silent divergence between what the user was shown and what actually gets signed" theme with the BatchForm duplicate-recipient issue and the EscrowForm self-escrow issue filed elsewhere in this batch.

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 | FWC26bugSomething isn't workingsecuritySecurity 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