hard: liquidity pool blindly pays any merchant address the creditline names - #96
hard: liquidity pool blindly pays any merchant address the creditline names#96BigMick03 wants to merge 1 commit into
Conversation
|
Please rebase/merge the base branch into your branch and resolve the conflicts — a fresh audit will run automatically once new commits land. |
EmeditWeb
left a comment
There was a problem hiding this comment.
❌ Automated Audit: does_not_solve
@BigMick03 Please look into the issue again and address the gaps below.
The authoritative verification flags an unresolved merge conflict, which by the audit rules means the PR cannot be approved regardless of content. Beyond that, the substantive defense is opt-in: all three mitigations (per-ledger outflow cap, per-merchant cap, vendor cross-check) default to DISABLED, so a freshly deployed pool remains exactly as vulnerable as before — a compromised creditline admin can still drain it until an admin manually configures caps and a vendor registry. The root cause (fund_loan blindly trusts the creditline-named merchant) is therefore not remediated in the default/initial state; this is defense-in-depth scaffolding, not a root-cause fix. The code itself is well-built and tested, but it does not actually close the described vulnerability out of the box.
Gaps identified:
- Unresolved merge conflict with base branch (hard blocker per audit rules)
- Caps/vendor-check default to disabled, so the vulnerability remains open in default deployment — root cause not remediated without out-of-band admin action
- No enforcement that caps or a vendor registry must be set before/at deployment (or sensible secure defaults) to actually bound drain risk
CI checks: ✅ PASSED: Build and Test Contracts
Merge conflicts:
Audited by stepfi-audit-bot 🤖
Summary
fund_loan()honoredtransfer(pool → merchant, amount)for whatever merchant the registered creditline supplied, with zero independent verification. The pool's solvency rested entirely on the creditline contract being bug-free; a compromised creditline admin could repointset_creditline()and drain the pool instantly, withabsorb_loss()erasing the accounting trace.This PR adds defense-in-depth caps to the liquidity pool, all disabled by default (backward compatible):
fund_loanoutflows within a single ledger bounded tomax_outflow_bpsof the available-liquidity snapshot (default10000= disabled; admin-only setter, rejects > 10000). New errorLedgerOutflowCapExceeded = 14.max_per_merchant(admin-only setter,0= disabled). New errorMerchantConcentrationCapExceeded = 15.set_vendor_registry(admin, Option<Address>); when set,fund_loanrequiresis_active(merchant)viatry_invoke_contractbefore transferring (fails closed). Unset = legacy behavior. New errorsVendorNotActive = 12,VendorNotRegistered = 13.(LQFUND, creditline, merchant), data(amount, remaining_ledger_cap, remaining_merchant_cap)for indexer monitoring.Semantics: caps default to disabled so honest flows are unchanged. Recommended production hardening:
set_max_outflow_bps(2500)(25%/ledger) +set_max_per_merchant(<ceiling>)after deployment, so a malicious creditline is bounded to a fraction of available liquidity per ledger instead of draining the pool instantly.Tests
18 new tests, 113 total in liquidity-pool-contract, all green: repeated-call outflow cap (rejection does not consume headroom), rolling-window reset on new ledger, default-disabled regression, setter validation + non-admin guards, per-merchant cap incl. isolation and rejected-attempt state, vendor check (approved/unregistered/suspended/unset/clear-roundtrip + non-admin guard), LQFUND event payload.
Verification:
cargo test -p liquidity-pool-contract→ 113 passed;cargo test --workspace→ 356 passed, 0 failed; clippy clean;wasm32-unknown-unknown --releasebuild succeeds.Split out of PR #95 per review feedback (parameters-contract multisig hardening stays in that PR).