[Cross-Layer] lending_pool redeem and deposit lack slippage bounds and virtual-share offset, permitting single-ledger share-price manipulation and value extraction

[ogazboiz]

Contributor coordination and questions: https://t.me/+DOylgFv1jyJlNzM0

Task Classification: Security defect remediation, cross-layer (audit-funded)
Affected Layers: contracts, backend, frontend (+ contracts)
Affected Paths: contracts/lending_pool/src/lib.rs, contracts/lending_pool/src/pool.rs, contracts/lending_pool/src/error.rs, contracts/lending_pool/src/test.rs, backend/src/services/poolQuoter.ts, backend/src/services/eventIndexer.ts, backend/src/routes/quotes.ts, backend/migrations/, frontend/src/hooks/useRedeemQuote.ts, frontend/src/components/RedeemForm.tsx, .github/workflows/ci.yml, docker-compose.yml
Severity: Critical (audit-funded tier)
Estimated Window: 72-96 hours

Technical Context & Monorepo Integration Failure

lending_pool mints and burns shares against a pooled asset. Current calc_shares_to_mint and calc_assets_to_return derive share price as total_assets / total_shares, where total_assets is read from token::Client::balance(pool_address) at call time, and neither deposit nor redeem accepts a client-supplied bound. Two properties follow: share price is a function of the live token balance, and a caller commits to whatever price exists when the invoking transaction executes. Both are exploitable within a single ledger because Soroban orders operations within a transaction set and a token transfer to the pool address mutates balance() without minting shares.

Concrete extraction walkthrough:

1. Pool state: total_shares = 0, contract token balance = 0.
2. Attacker calls deposit(attacker, 1) and receives 1 share (first-depositor path, shares = assets when supply is zero).
3. Attacker transfers 1_000_000 of the asset directly to the pool address via the token contract. balance() is now 1_000_001; total_shares is still 1. Share price jumped to 1_000_001 assets per share with no supply change.
4. Victim deposit(victim, 1_000_000) computes shares = 1_000_000 * 1 / 1_000_001 = 0 after integer floor. The victim receives 0 shares while the balance rises to 2_000_001.
5. Attacker redeem(attacker, 1) computes assets = 1 * 2_000_001 / 1 = 2_000_001 and drains the victim's deposit plus the donation.

A contract-only bound (reject shares == 0) blocks step 4 but not the general case where the victim receives a nonzero but under-valued share count, and it cannot express the caller's price expectation. A backend-only quote is advisory and cannot bind on-chain settlement. A frontend-only guard is bypassed by any direct RPC caller. The invariant must be enforced in lending_pool, quoted by backend, and surfaced by frontend, because the trust boundary (the settling contract), the pricing oracle (indexed events plus simulation), and the user's tolerance input live in three separate layers. Drift between the price the backend quotes and the price the contract settles at is itself the attack surface, so the same accounting definition must be shared across all three.

Core Component Invariants & Code Paths

Smart Contract Infrastructure. Replace balance-derived pricing with internal accounting. Add DataKey::TotalManagedAssets (persistent) mutated only inside deposit, redeem, and accrual paths; never read token.balance for pricing. Introduce constants VIRTUAL_SHARES and VIRTUAL_ASSETS (decimals offset, 10^OFFSET with OFFSET = 3) applied in both directions:

• calc_shares_to_mint(assets) returns assets * (total_shares + VIRTUAL_SHARES) / (total_managed_assets + VIRTUAL_ASSETS), rounded down.
• calc_assets_to_return(shares) returns shares * (total_managed_assets + VIRTUAL_ASSETS) / (total_shares + VIRTUAL_SHARES), rounded down.

Extend signatures to deposit(env, from, assets, min_shares_out) and redeem(env, from, shares, min_assets_out); add read-only preview_deposit and preview_redeem returning the same math without state change. Add PoolError::MinSharesNotMet, PoolError::MinAssetsNotMet, PoolError::ZeroShares, PoolError::InsufficientLiquidity. Emit an accounting event on every mutation: env.events().publish((symbol_short!("price"),), (ledger_seq, total_managed_assets, total_shares)), plus existing deposit and redeem topics. Post-change invariants: pricing is independent of unsolicited transfers (donation resistance); mint rounds shares down and redeem rounds assets down, both in the pool's favor; a settlement failing its bound reverts with a typed error rather than executing at a shifted price.

Backend/API Layer. Add backend/src/routes/quotes.ts exposing GET /api/pools/:poolId/quote?action=redeem|deposit&amount=<i128>. backend/src/services/poolQuoter.ts reads pool storage via Soroban RPC getLedgerEntries, calls simulateTransaction against preview_redeem/preview_deposit, and returns { expected, min_bound, assets_per_share, ledger_seq, ttl_ms } where min_bound = expected * (10_000 - slippage_bps) / 10_000. Extend eventIndexer.ts (Soroban RPC getEvents) to ingest the price topic into a new table so quotes reconcile against the last settled price and reject on ledger skew. Post-change invariants: the quote's math is byte-for-byte the contract math (shared fixed-point rounding); a quote carries the ledger_seq it was computed at; the endpoint returns 409 when the indexed head has advanced past the simulated ledger.

Frontend Client. frontend/src/hooks/useRedeemQuote.ts (TanStack Query) fetches the quote and subscribes to the existing SSE stream to invalidate on new price events. frontend/src/components/RedeemForm.tsx computes min_assets_out from NEXT_PUBLIC_DEFAULT_SLIPPAGE_BPS, blocks submission when the live quote deviates beyond tolerance, and passes the bound into the contract invocation. New env vars: NEXT_PUBLIC_DEFAULT_SLIPPAGE_BPS, NEXT_PUBLIC_QUOTE_TTL_MS. Post-change invariants: no transaction is signed without a min_assets_out/min_shares_out derived from a quote younger than QUOTE_TTL_MS; an SSE price update stale-marks the form and forces re-quote.

Root Configuration & Orchestration. Add a cross-layer property-test target and an e2e container job to .github/workflows/ci.yml, invoked per layer (no root workspace exists). The e2e job uses docker-compose.yml to stand up postgres:16, the backend, and a local Soroban network, then runs the round-trip extraction scenario against real settlement. New migration file registered under backend/migrations and gated by the existing migration-check job. Post-change invariant: the extraction walkthrough above is a failing-then-passing regression fixture executed by root-initiated CI.

Value-conservation invariant (adversarial ordering). For any operation sequence within one ledger, let A = total_managed_assets, S = total_shares. Define P = (A + VIRTUAL_ASSETS) / (S + VIRTUAL_SHARES). The following must hold under any attacker-chosen ordering including interleaved direct token transfers:

• Donation independence: a raw token transfer to the pool leaves A, S, and P unchanged.
• Round-trip non-profitability: deposit(d) immediately followed by redeem(minted_shares) yields assets_out <= d.
• Monotonic price: P is non-decreasing except through explicit accrual or loss-realization paths; no mint/burn pair lowers P.
• Bound safety: every settlement satisfies assets_out >= min_assets_out and shares_out >= min_shares_out or reverts.

Verification & Acceptance Criteria

• Root-initiated compilation: cargo build --manifest-path contracts/Cargo.toml (overflow-checks on), npm --prefix backend run build, npm --prefix frontend run build all pass.
• cargo test in contracts/lending_pool includes: donation-independence case, first-depositor virtual-offset case, redeem/deposit bound rejection returning PoolError::MinAssetsNotMet/MinSharesNotMet, and a property test over randomized sequences asserting round-trip non-profitability.
• Backend npm --prefix backend test covers poolQuoter math parity against contract simulation and the 409 ledger-skew path; npm --prefix backend run typecheck and lint pass.
• Migration applies and reverts: migrate:up then migrate:down complete without error under the migration-check job; no conflict with existing migration ordering.
• Frontend test:e2e (Playwright) asserts the form blocks submission on stale quote and injects a nonzero min_assets_out.
• E2e container job reproduces the five-step extraction scenario and asserts the attacker redeem reverts.
• PR attachments: pre-fix and post-fix pool state dumps (total_managed_assets, total_shares, assets_per_share) for the extraction scenario; simulation-vs-settlement diff showing zero drift.

Suggested Execution Path

Phase 1 - Contract accounting, virtual offset, slippage params (24-32h). Deliverables: internal TotalManagedAssets accounting; VIRTUAL_SHARES/VIRTUAL_ASSETS in both calc paths; extended deposit/redeem signatures with bounds; preview_* functions; new PoolError variants; price event. Exit check: contract property tests and the donation/first-depositor cases pass under overflow-checks=true.

Phase 2 - Backend quote endpoint, indexer, migration (18-24h). Deliverables: poolQuoter.ts, quotes.ts route, eventIndexer.ts price ingestion, share-price migration. Exit check: quote math matches contract simulation byte-for-byte; migrate:up/migrate:down complete without error; typecheck, lint, test green.

Phase 3 - Frontend slippage guard (14-18h). Deliverables: useRedeemQuote hook, RedeemForm bound derivation, SSE-driven re-quote, new NEXT_PUBLIC_* env vars. Exit check: test:e2e proves no signature without a fresh bound.

Phase 4 - Root cross-layer invariant tests and CI (16-22h). Deliverables: e2e compose job, per-layer property-test wiring in ci.yml, extraction regression fixture, state-dump attachments. Exit check: root-initiated CI runs the extraction scenario red on the pre-fix commit and green on the branch.

[midenotch]

I've reviewed the issue and understand the requirements. I'd love to work on this and submit a clean PR that meets the acceptance criteria.

[Naajih09]

Hi, I'd like to work on this issue. I've gone through the description and acceptance criteria and I'm confident I can deliver it. I'll make sure every acceptance criterion is met and submit a clean, tested PR. Please assign it to me.

[Akpolo]

I'd like to pick this one up.

This is the classic ERC-4626-style inflation attack, just on Soroban instead of the EVM — first depositor mints 1 share, donates directly to the pool address to blow up the share price, then the victim's deposit floors to zero shares and the attacker redeems the lot. I've spent a lot of time in this exact territory. I build Soroban contracts in Rust (escrow and vault-style accounting included), and I also work in TypeScript backends and Next.js, which matters here since the fix genuinely can't live in one layer. The contract enforces, the backend quotes, the frontend carries the user's tolerance — and if any of the three computes price differently, that gap is the exploit.

The heart of it is getting pricing off token.balance(). Once the pool tracks its own TotalManagedAssets and only mutates it through deposit/redeem/accrual, a donation just becomes dead weight sitting in the contract — it can't move the price. The virtual offset kills the first-depositor trick, rounding down on both mint and redeem keeps the dust in the pool's favor, and the min_shares_out/min_assets_out bounds mean a caller commits to a price up front instead of eating whatever the ledger gives them. On the backend side the thing I'd be strict about is math parity — the quoter should get its numbers from simulating preview_deposit/preview_redeem, not from reimplementing the formula in JS, because a reimplementation will eventually drift and drift is the attack surface. Same reason the 409-on-ledger-skew check matters. Frontend just refuses to sign without a fresh bound.

Timeline-wise the phased plan in the issue is sensible and I'd follow it as written — contract accounting and the property tests first (donation independence, first-depositor case, round-trip non-profitability, all under overflow-checks), then quoter/indexer/migration, then the slippage guard, then the CI job that replays the five-step extraction and proves it reverts. I can work within the 72–96h window and I'd have the Phase 1 contract changes up early so you can check the accounting approach before the rest stacks on top of it.

[grantfox-oss]

🦊 GrantFox — @Akpolo has been assigned to this issue as part of the Official Campaign | FWC26 campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #1380)
2. Your PR will be reviewed by the LabsCrypt maintainers

Good luck! Track your progress on GrantFox.