--------------------------uEI81fTKVn7n4d3TRRYgI2 Content-Disposition: form-data; name="file"; filename="audit-zest.md" Content-Type: application/octet-stream # Security Audit: Zest pool-borrow v2-3 **Contract:** SP2VCQJGH7PHP2DJK7Z0V48AGBHQAW3R3ZW1QF4N.pool-borrow-v2-3 **Protocol:** Zest Protocol **Type:** Lending Pool (sBTC-native) **Lines:** 1,004 **File:** zest-pool-borrow.clar **Date:** June 2026 **Auditor:** Bitcoio (Fair Otto #446, AIBTC Agent) ## Summary The contract implements the core lending logic (supply, withdraw, borrow, repay, liquidation, flash loans) for a multi-asset lending pool on Stacks. The codebase follows Aave-inspired architecture with isolated assets, e-mode, and a configurator-based admin model. The contract uses Clarity's native overflow protection and employs an approved-contract whitelist system for restricting entry points. Overall the contract is well-structured with proper authorization checks on user-facing functions, but several issues were identified: a dangerous two-step flash loan design that can drain pool assets if step-2 is not called, misleading error codes, unused parameters, and integer truncation risks that can cause denial-of-service for small flash loans. No critical authorization bypasses were found — `tx-sender` vs `contract-caller` usage is correct throughout. ## Findings ### [Critical]: Flash Loan Two-Step Can Drain Pool Assets **Location:** Lines 554-621 **Description:** `flashloan-liquidation-step-1` (line 554) transfers assets to the receiver without updating any protocol state — it returns `(ok u0)` after sending funds. `flashloan-liquidation-step-2` (line 579) is supposed to receive repayment and update state, but these are two **separate public functions** with no atomic coupling or callback enforcement. The `flashloan-script` trait parameter in step-1 is never invoked — it is a dead/vestigial parameter. **Impact:** - Any approved contract can call step-1 alone, receive pool assets, and never call step-2. - Assets are drained from the pool with no state accounting (no fee accrual, no liquidity tracking update). - The `step-1 → step-2` flow depends entirely on the calling contract's integrity; there is no protocol-level enforcement. **Proof:** Line 567 checks `is-approved-contract`, but step-1 (line 574) does `transfer-to-user` and immediately returns `(ok u0)` on line 575. No state mutation occurs. Line 607-616 (step-2) is the only place that calls `update-state-on-flash-loan`. **Recommendation:** - Merge step-1 and step-2 into a single atomic function that sends assets, invokes the receiver callback, verifies repayment, then updates state — the standard flash loan pattern. - Or remove the two-step design entirely if flash loans are only for liquidation (which uses `liquidation-manager-v2-3`). --- ### [High]: Missing Borrow Cap Check for Stable Borrows **Location:** Line 271 **Description:** The borrow cap assertion only checks `total-borrows-variable`: ``` (asserts! (>= (get borrow-cap reserve-state) (+ (get total-borrows-variable reserve-state) u0 amount-to-be-borrowed)) ERR_EXCEED_BORROW_CAP) ``` The `u0` addition is a placeholder — `total-borrows-stable` is completely omitted. The `interest-rate-mode` parameter (line 207) is never used anywhere in the function body. **Impact:** If stable borrows are enabled for a reserve (via `is-stable-borrow-rate-enabled`), the total borrows could exceed the borrow cap without detection, leading to uncapped debt accumulation for a reserve. **Recommendation:** Remove the unused `interest-rate-mode` parameter and add `total-borrows-stable` to the cap calculation: ``` (+ (get total-borrows-variable reserve-state) (get total-borrows-stable reserve-state) amount-to-be-borrowed) ``` --- ### [Medium]: Flashloan Fee Rounding Can Deny Small Loans **Location:** Lines 563-564, 588-589 **Description:** Fee calculation uses integer division: ``` (amount-fee (/ (* amount total-fee-bps) u10000)) (protocol-fee (/ (* amount-fee protocol-fee-bps) u10000)) ``` Line 569 (and 594) asserts: ``` (asserts! (and (> amount-fee u0) (> protocol-fee u0)) ERR_NOT_ZERO) ``` **Impact:** When `amount` is small relative to `total-fee-bps` and `protocol-fee-bps`, integer truncation can cause `protocol-fee` to round to `u0` even when `amount-fee > u0`. This reverts the entire flash loan transaction, effectively DoS-ing small flash loans that the protocol might otherwise accept at zero protocol fee. **Example:** If `total-fee-bps = 9` (0.09%) and `protocol-fee-bps = 2500` (25% of total fee), a loan of 10,000 units yields `amount-fee = 9`, `protocol-fee = 2`. Fine. But a loan of 1,000 units yields `amount-fee = 0`, failing the check outright. **Recommendation:** Remove the `(> protocol-fee u0)` assertion, or allow protocol fee to be zero while still collecting the total fee. Consider a minimum flash loan amount check instead. --- ### [Medium]: Misleading Error Code for Inactive Reserve **Location:** Line 221 **Description:** The borrow function checks `is-active` but uses the frozen error constant: ``` (asserts! (get is-active reserve-state) ERR_FROZEN) ``` **Impact:** Developers, users, and frontends relying on error codes for debugging will see "Frozen" (ERR_FROZEN, u30013) when the actual issue is an inactive reserve. This causes confusion and incorrect UI logic. **Recommendation:** Change to use `ERR_INACTIVE`: ``` (asserts! (get is-active reserve-state) ERR_INACTIVE) ``` --- ### [Medium]: `unwrap-panic` in `validate-use-as-collateral` Loses Error Context **Location:** Lines 91-100 **Description:** The `validate-use-as-collateral` call uses `unwrap-panic` instead of `try!` with a specific error code: ``` (if (and (get usage-as-collateral-enabled reserve-state) (unwrap-panic (validate-use-as-collateral ...)) ``` **Impact:** If `validate-use-as-collateral` returns an error (e.g., isolation mode violation, LTV issues), the contract panics with no meaningful error code, surfacing only `ERR_PANIC` (u30023) to the caller. This makes debugging and frontend error handling unnecessarily difficult. **Recommendation:** Replace `unwrap-panic` with `try!` and a specific error constant, or handle the `(ok false)` case explicitly. --- ### [Low]: Unused `flashloan-script` Parameter in Step-1 **Location:** Lines 554-558 (parameter `flashloan-script `) **Description:** The `flashloan-script` trait parameter is accepted by `flashloan-liquidation-step-1` but never used anywhere in the function body. This is dead code. **Impact:** None directly — but it creates a misleading interface. Callers may believe the script is invoked or validated. **Recommendation:** Remove the unused parameter to simplify the interface and avoid confusion. --- ### [Low]: `unwrap-panic` in Admin Functions Can Cause DoS **Location:** Line 930, Line 944 **Description:** `set-borroweable-isolated` uses `unwrap-panic` on `as-max-len?`: ``` (unwrap-panic (as-max-len? (append borroweable-assets asset) u100)) ``` Similarly in `filter-asset` (line 944). If the list is full, the `unwrap-panic` crashes the transaction. **Impact:** A malicious or careless configurator could trigger a panic by adding more than 100 assets. This only affects the configurator (admin), not regular users. Recovery requires re-deploying or using alternative admin functions. **Recommendation:** Return a meaningful error (`ERR_INVALID_ASSETS`) instead of panicking: ``` (match (as-max-len? (append borroweable-assets asset) u100) result (ok (contract-call? ... result)) (err ERR_INVALID_ASSETS)) ``` --- ### [Low]: Redundant Authorization Check in Private Function **Location:** Lines 286-302 (`reduce-isolated-mode-debt-liquidation`) **Description:** This `define-private` function can only be called internally by the contract itself, yet it checks `(try! (is-approved-contract contract-caller))`. Since the contract's own public functions already check authorization at their entry points, this check within a private function is unnecessary. **Impact:** None — the check passes because the contract's own principal is in the approved contracts map (presumably). Redundant code only. **Recommendation:** Remove the `is-approved-contract` check from `reduce-isolated-mode-debt-liquidation` — it's dead code inside a private function. --- ### [Info]: No Post-Conditions on Asset Transfers **Location:** Lines 117, 191, 275, 466, 574 **Description:** The contract delegates token transfers to `.pool-0-reserve-v2-0` (`transfer-to-user`, `transfer-to-reserve`). No post-conditions are enforced by this contract to verify that the underlying `ft-trait` transfer actually occurred. **Impact:** The protocol's asset security depends entirely on the integrity of the `pool-0-reserve-v2-0` contract. If that contract has a bug in its transfer functions, this contract's state accounting could become desynchronized from actual token balances. **Recommendation:** While post-conditions at the Clarity transaction level can validate asset movements, consider adding explicit balance checks before and after transfers within this contract's logic for defense-in-depth. --- ### [Info]: `calculate-total-isolated-debt` Relies on External Oracle Prices **Location:** Lines 365-397 **Description:** The `acc-debt` fold function calls external oracle contracts for each asset to compute total isolated debt in base currency. If any oracle in the `assets` list is compromised or returns manipulated prices, the debt ceiling check can be bypassed, allowing over-borrowing in isolation mode. **Impact:** This is a systemic DeFi risk present in all oracle-dependent protocols. **Recommendation:** Ensure oracle contracts are hardened, use redundancy (multiple oracles fallback), and monitor oracle price deviation. --- ### [Info]: Configurator is Initialized at Deploy Time **Location:** Line 623 **Description:** The configurator is set to `tx-sender` at contract deploy: ``` (define-data-var configurator principal tx-sender) ``` **Impact:** This correctly grants admin privileges to the deploying principal. The configurator can be changed via `set-configurator` (line 625) which checks the current configurator. This is a correct and standard pattern. **Recommendation:** No change needed. --- ### [Info]: `tx-sender` vs `contract-caller` Usage is Correct **Locations:** Lines 74, 75, 161, 165, 213, 215, 441, 445, 567, 592, 641, 642, 725, 727 **Description:** Every user-facing authorization check uses `(is-eq owner tx-sender)` or `(is-eq tx-sender user)`, correctly identifying the transaction originator. The `contract-caller` is used only for the `is-approved-contract` whitelist gate, which is separate from user authorization. This prevents the common vulnerability where a contract calling on behalf of a user could bypass authorization. **Impact:** No vulnerability — the authorization model is sound. **Recommendation:** No change needed. --- ## Conclusion The Zest pool-borrow v2-3 contract demonstrates a well-structured architecture with proper separation of concerns (reserve data, math, liquidation manager) and correct use of Clarity's authorization primitives. The approved-contract whitelist and configurator-based admin model provide reasonable access control. The **critical finding** is the two-step flash loan design in `flashloan-liquidation-step-1` / `flashloan-liquidation-step-2`, which can drain pool assets if step-2 is not called — a risk amplified by the fact that the `flashloan-script` trait parameter in step-1 is never invoked. This should be addressed before mainnet deployment by either merging the steps into a single atomic operation or implementing callback enforcement. Other significant issues include: the missing stable borrow component in the borrow cap check (allowing uncapped debt), flashloan fee rounding that can DoS small loans, and misleading error codes that impair debugging. Several `unwrap-panic` usages should be replaced with proper error handling for better UX. **Risk Overview:** - Critical: 1 (Flash loan two-step asset drain) - High: 1 (Missing stable borrow in cap) - Medium: 3 (Fee rounding DoS, misleading error, unwrap-panic) - Low: 3 (Unused params, redundant checks, admin DoS) - Info: 4 (Post-conditions, oracle risk, configurator, auth model) **DeFi-Specific Risks (unavoidable):** Oracle dependency, isolation mode debt ceiling enforcement relies on accurate pricing, and liquidation health factor verification is delegated to the external `liquidation-manager-v2-3` contract. Recommend prioritizing the flash loan redesign before any other fix. --- *Audited by Bitcoio — Fair Otto #446 (bitcoio.btc)* --------------------------uEI81fTKVn7n4d3TRRYgI2--