Skip to content

sec(api): approvePurchaseViaSession leaks execution existence and status before authorization #172

Closed

Description

@cristim

Found while adversarially reviewing PR LeanerCloud/cloud-commitments-cli#1744. Pre-existing on main, outside that diff, so filed rather than folded in.

approvePurchaseViaSession returns a 409 carrying status=<X> before calling authorizeSessionApprove. A session with no approve rights can therefore probe both the existence and the current status of an execution whose UUID it already holds.

Low severity, and deliberately labelled as such: it requires knowing the UUID, and it discloses status rather than content. But it is an authorization check running after a response that varies on protected state, which is the shape worth not having.

For contrast, a sibling path was checked and is fine: runPlannedPurchase validates UUID syntax before authorizing, which discloses nothing — a malformed UUID is malformed regardless of what exists. The distinction is whether the pre-auth response varies with protected state. It does in the 409 case and does not in the syntax case.

Suggested fix

Authorize before any response that depends on the execution row. If a distinct 409 is genuinely useful to legitimate callers, emit it only after the approve check passes; unauthorized callers should get the same response whether or not the execution exists.

Verification

Two directions, both required:

  • An unauthorized session probing a real execution UUID and a non-existent one must get responses that are indistinguishable — same status, same body.
  • An authorized session must still receive the informative 409, or the fix has traded a small leak for a real usability regression.

A test asserting only the first direction would pass against a handler that refuses everyone.

Related

Findings from the 2026-09-02 codebase audit

Added by an automated audit of 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd (tip of origin/main). Each item below was reported by one reviewer and independently confirmed by a second that did not write it. Full report: docs/audits/codebase-audit-2026-09-02.md.

A01-017 (low)

The same shape on two more routes, and one auth level lower, from finding A01-017. This issue covers approvePurchaseViaSession; the other AuthPublic money routes leak the same way to a caller with no token and no session at all. revokeViaEmailToken (internal/api/handler_purchases.go:1248-1260) returns 404 "execution not found", 409 "still pending" or 409 "cannot be revoked (status=)" before tryRevokeViaSession or authorizeApprovalAction run; cancelPurchase (:1029-1035) 404s pre-auth; and loadApproveExecution (:523-537) returns 404 and orphan-detail 409s before either auth branch. All three are registered as AuthPublic at router.go:163-172. The verification bar stated here (a real and a non-existent UUID must be indistinguishable) should apply to all four handlers, since fixing one leaves the oracle intact on the others.

Activity

  1. IAmJSD commented on Oct 20, 2026

    @IAmJSD
    Collaborator

    Claiming: working on this

  2. self-assigned this
    on Oct 20, 2026
  3. IAmJSD commented on Oct 20, 2026

    @IAmJSD
    Collaborator

    Closing as already fixed on main, via three PRs that each closed a different issue:

    Checked on current main with a throwaway mock-based probe through HandleRequest:

    • A session with no RBAC, an approve-own non-creator and an out-of-scope session (all with CSRF and no token), against completed, approved, canceled and failed executions, all got 404 execution not found, identical to a missing ID.
    • An authorized admin session still gets the descriptive 409 ... (status=completed).
    • TestHandleRequest_TokenActions_SignedInCannotTellMissingFromExisting passes.

    Same pattern in a sibling path, not covered here:

    • In fetchAndAuthorizeRIExchange (internal/api/handler_ri_exchange.go), the 409 "exchange %s cannot be approved (status=%s)" is returned before the creator check.
    • So an in-scope approve-own holder can learn the status of another user's exchange, where a missing one would get a 404.
    • Low severity, since it needs approve-own and must be in scope. I can file it separately if you want it tracked.

    @cristim, FYI.

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

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions