Skip to content

send_payment and create_payment_request accept unbounded memo strings despite a doc comment recommending a 28-byte maximum #49

Description

@abayomicornelius

Overview

send_payment's doc comment sets an explicit expectation for the memo field:

// stellar_send/src/lib.rs:181-193
/// * `memo`   – Arbitrary memo string (max 28 bytes recommended).
pub fn send_payment(
    env: Env,
    from: Address,
    to: Address,
    token: Address,
    amount: i128,
    memo: String,
) -> Result<PaymentRecord, StellarSendError> {

Nothing in the function body enforces that recommendation. memo: String (a Soroban soroban_sdk::String) flows straight through to storage with zero length check:

// stellar_send/src/lib.rs:217-231
let seq = Self::next_seq(&env);
let record = PaymentRecord {
    from: from.clone(),
    to: to.clone(),
    token: token.clone(),
    net_amount,
    fee_amount,
    memo: memo.clone(),
    ...
};
let key = (from.clone(), seq);
env.storage().persistent().set(&key, &record);

The same is true of create_payment_request's memo field (stellar_send/src/payment_request.rs:52-89, parameter documented only as free text with no length constraint mentioned or enforced). send_batch_payment and execute_subscription/path-payment records are unaffected since they construct their own fixed, short memo strings internally ("batch_payment", "path_payment") rather than accepting caller input for this field.

The "28 bytes recommended" figure isn't arbitrary — it mirrors the well-known 28-byte limit on Stellar's own classic-transaction MEMO_TEXT field, which is presumably where this doc comment's guidance comes from. But because it's only a comment, not an enforced constraint, a caller can pass an arbitrarily large String (bounded only by the overall transaction/footprint resource limits Soroban enforces at the protocol level, which are far larger than 28 bytes) as memo on every send_payment or create_payment_request call. Each such record is written to persistent() storage and kept indefinitely (there's no cleanup/expiry of settled PaymentRecord/PaymentRequest entries). This is a storage-cost footgun in two directions: (1) an honest integrator who doesn't realize the "recommended" limit isn't actually enforced can accidentally bloat their own storage-rent costs by passing large structured data (e.g. JSON) through memo expecting it to be capped, and (2) it's a cheap, uncapped vector for anyone to inflate the protocol's aggregate persistent-storage footprint across many calls, since nothing about the memo length scales with, or is bounded relative to, the payment amount itself — a payment of 1 unit can carry a memo many times larger than the recommended limit at the same cost as a memo-free equivalent.

Requirements

  • Enforce a maximum memo length in both send_payment and create_payment_request, matching the documented 28-byte recommendation (or whatever limit is actually decided on — the point is that the doc comment and the enforced behavior need to agree), returning a new or existing StellarSendError variant (InvalidAmount isn't quite right semantically; a new InvalidMemo or reused validation-style variant is more appropriate) when exceeded.
  • Apply the same limit consistently to both call sites, since they share the same documented rationale.
  • Double check whether String::len() (or the equivalent Soroban String length accessor) measures bytes or some other unit, to make sure the enforced limit actually matches the "28 bytes" the doc comment promises rather than, say, 28 UTF-8 code points which could differ for non-ASCII memos.

Acceptance Criteria

  • send_payment rejects a memo longer than the documented limit with a clear, typed error.
  • create_payment_request rejects an over-long memo the same way.
  • test_send_payment_rejects_oversized_memo and test_create_payment_request_rejects_oversized_memo (or equivalently named) added.
  • A boundary test confirms a memo at exactly the limit still succeeds (off-by-one correctness).

Additional Notes

Precise references: stellar_send/src/lib.rs:185 (the "max 28 bytes recommended" doc comment), :192, 225 (the unconstrained memo: String parameter and its unchecked pass-through into PaymentRecord), stellar_send/src/payment_request.rs:40, 58, 81 (the equivalent unconstrained memo field/parameter on PaymentRequest/create_payment_request).

Why this is a storage-cost concern and not just a cosmetic doc/code mismatch: Soroban's persistent-storage pricing is rent-based and scales with entry size; a PaymentRecord/PaymentRequest with an oversized memo costs more to keep alive than one within the documented bound, and — combined with the already-open "no maximum batch size" and the general lack of any per-caller rate limiting anywhere in this workspace — an unenforced memo cap is one more small, currently-uncapped lever available to anyone wanting to cheaply bloat the protocol's aggregate storage footprint relative to the number of genuine payments processed.

Test/reproduction plan: in stellar_send/src/test.rs, using the existing setup()/mint() helpers and String::from_str: call send_payment with a memo built from, say, a 500-byte repeated-character string, and confirm it succeeds and is stored verbatim on main today (record.memo.len() far exceeding 28). After the fix, assert the same call is rejected with the new validation error, and add a boundary case at exactly the chosen limit (e.g. 28 bytes) confirming it still succeeds.

Metadata

Metadata

Assignees

Labels

GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardOfficial Campaign | FWC26Campaign: Official Campaign | FWC26Third CampaignCampaign: Third CampaignbugSomething isn't workingcontractsSmart contract logicvery 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