Background / Context
Cargo.toml's [profile.release] sets overflow-checks = true and panic = "abort". This means any arithmetic overflow anywhere in the contract's arithmetic (pool.balance += amount, milestone.remaining_budget -= amount, total * fee_bps / BPS_DENOMINATOR, distributable * bps / BPS_DENOMINATOR, allocated += share, etc.) causes an immediate panic/abort — a full transaction failure, not a graceful error return. In Soroban, a panicking contract invocation fails the whole transaction, so this is effectively a denial-of-service vector against a specific escrow/milestone/pool if triggerable by an adversary rather than only by pathological legitimate use.
Problem Statement
Walk every arithmetic expression in contracts/escrow/src/lib.rs, contracts/milestones/src/lib.rs, and contracts/maintenance-pool/src/lib.rs and determine: (1) which are reachable with attacker- or sponsor-controlled inputs (amount, total_budget, repeated deposit/allocate calls accumulating toward i128::MAX), (2) whether any panic path can be triggered by someone other than the party who would be harmed by the panic (e.g., can a third party cause pool.balance += amount to overflow via a huge single deposit, bricking withdraw for the pool's legitimate maintainers, i.e., a DoS against other sponsors' funds already in the same pool)? i128::MAX is astronomically large in absolute terms, but Stellar tokens can have configurable decimals, and total_deposited/total_withdrawn are monotonically-accumulating counters that never decrease — a sufficiently long-lived, high-volume maintenance pool is a realistic path to large accumulated values worth modeling precisely, not dismissing.
Requirements
- Produce a call-graph-level audit table: expression, file:line, operands' provenance (attacker-controlled / sponsor-controlled / accumulated-over-time), and overflow reachability verdict.
- For any expression judged realistically reachable (even over a multi-year time horizon, given
total_deposited/total_withdrawn never shrink), implement checked arithmetic (checked_add/checked_mul/checked_sub) that returns a proper Error variant instead of panicking, preserving the "no fund loss, no bricked pool" property.
- For expressions judged unreachable, add an explicit comment justifying why, plus a test using extreme values (e.g., deposits summing near
i128::MAX) proving the code behaves as expected (either succeeds correctly or fails gracefully, never panics uncontrolled).
- Add fuzz or property tests specifically targeting arithmetic boundaries.
Acceptance Criteria
Technical Notes / Hints
- Key accumulation points:
maintenance-pool/src/lib.rs:84-87 (pool.balance += amount, pool.total_deposited += amount), milestones/src/lib.rs:110 (milestone.remaining_budget -= amount), compute_split's fee/distributable/allocated arithmetic in both escrow and milestones.
fee_bps is a u32 cast to i128 at multiple sites — verify the cast itself can't be a vector (it can't overflow going u32→i128, but check multiplication total * fee_bps for large total).
Difficulty Justification
Requires systematically reasoning about a panic=abort failure mode's real-world exploitability across three contracts and many call sites, distinguishing "theoretically possible with i128" from "actually reachable given realistic Stellar token decimal/volume assumptions over realistic pool lifetimes," and implementing checked-arithmetic error handling without silently changing success-path behavior anywhere.
Background / Context
Cargo.toml's[profile.release]setsoverflow-checks = trueandpanic = "abort". This means any arithmetic overflow anywhere in the contract's arithmetic (pool.balance += amount,milestone.remaining_budget -= amount,total * fee_bps / BPS_DENOMINATOR,distributable * bps / BPS_DENOMINATOR,allocated += share, etc.) causes an immediate panic/abort — a full transaction failure, not a graceful error return. In Soroban, a panicking contract invocation fails the whole transaction, so this is effectively a denial-of-service vector against a specific escrow/milestone/pool if triggerable by an adversary rather than only by pathological legitimate use.Problem Statement
Walk every arithmetic expression in
contracts/escrow/src/lib.rs,contracts/milestones/src/lib.rs, andcontracts/maintenance-pool/src/lib.rsand determine: (1) which are reachable with attacker- or sponsor-controlled inputs (amount,total_budget, repeateddeposit/allocatecalls accumulating towardi128::MAX), (2) whether any panic path can be triggered by someone other than the party who would be harmed by the panic (e.g., can a third party causepool.balance += amountto overflow via a huge single deposit, brickingwithdrawfor the pool's legitimate maintainers, i.e., a DoS against other sponsors' funds already in the same pool)?i128::MAXis astronomically large in absolute terms, but Stellar tokens can have configurable decimals, andtotal_deposited/total_withdrawnare monotonically-accumulating counters that never decrease — a sufficiently long-lived, high-volume maintenance pool is a realistic path to large accumulated values worth modeling precisely, not dismissing.Requirements
total_deposited/total_withdrawnnever shrink), implement checked arithmetic (checked_add/checked_mul/checked_sub) that returns a properErrorvariant instead of panicking, preserving the "no fund loss, no bricked pool" property.i128::MAX) proving the code behaves as expected (either succeeds correctly or fails gracefully, never panics uncontrolled).Acceptance Criteria
Errorvariants where reachable, across all three contractsi128::MAXscenarios) added to each contract's test modulecargo test --workspaceand wasm build both greenTechnical Notes / Hints
maintenance-pool/src/lib.rs:84-87(pool.balance += amount,pool.total_deposited += amount),milestones/src/lib.rs:110(milestone.remaining_budget -= amount),compute_split'sfee/distributable/allocatedarithmetic in bothescrowandmilestones.fee_bpsis au32cast toi128at multiple sites — verify the cast itself can't be a vector (it can't overflow going u32→i128, but check multiplicationtotal * fee_bpsfor largetotal).Difficulty Justification
Requires systematically reasoning about a
panic=abortfailure mode's real-world exploitability across three contracts and many call sites, distinguishing "theoretically possible with i128" from "actually reachable given realistic Stellar token decimal/volume assumptions over realistic pool lifetimes," and implementing checked-arithmetic error handling without silently changing success-path behavior anywhere.