diff --git a/Clarinet.toml b/Clarinet.toml index e1df787..36d32a1 100644 --- a/Clarinet.toml +++ b/Clarinet.toml @@ -359,6 +359,16 @@ path = "contracts/test/flashstack-stx-pool-v3.clar" clarity_version = 3 epoch = "3.0" +[contracts.test-stx-pool-v3-receiver-reentrant] +path = "contracts/test/test-stx-pool-v3-receiver-reentrant.clar" +clarity_version = 3 +epoch = "3.0" + +[contracts.test-sbtc-pool-v3-receiver-reentrant] +path = "contracts/test/test-sbtc-pool-v3-receiver-reentrant.clar" +clarity_version = 3 +epoch = "3.0" + [contracts.flashstack-sbtc-core-v2] path = "contracts/test/flashstack-sbtc-core-v2.clar" clarity_version = 3 diff --git a/contracts/flashstack-sbtc-pool-v3.clar b/contracts/flashstack-sbtc-pool-v3.clar index ecf3f64..daa7819 100644 --- a/contracts/flashstack-sbtc-pool-v3.clar +++ b/contracts/flashstack-sbtc-pool-v3.clar @@ -5,6 +5,10 @@ ;; Here the update only PROPOSES; the new admin must call accept-admin. NOT DEPLOYED. ;; F-8 FIX -- deposit is also gated by pause now (v2's deposit was not; flash-loan was). ;; See docs/security/FINDINGS_REGISTER.md. withdraw stays ungated so LPs can always exit. +;; F-9 FIX -- reentrancy lock on deposit/withdraw/flash-loan, ported from +;; flashstack-pool-v3's pv3-F1 guard. Without it, a flash-loan receiver can +;; "repay" by calling deposit mid-callback and mint shares at the loan-depressed +;; price, diluting every LP (proven on the live v2 pools, ajv.4.8). ;; ============================================================================ ;; FlashStack sBTC Pool v2 (HARDENED) ;; @@ -52,6 +56,7 @@ (define-constant ERR-NO-SHARES (err u708)) (define-constant ERR-INSUFFICIENT-SHARES (err u709)) (define-constant ERR-TRANSFER-FAILED (err u710)) +(define-constant ERR-REENTRANT (err u712)) ;; ============================================= ;; State @@ -68,6 +73,12 @@ (define-data-var total-volume uint u0) (define-data-var total-fees uint u0) +;; F-9 fix: one lock shared by deposit/withdraw/flash-loan. This pool holds a +;; single asset, so a bool is the per-asset lock (pool-v3 needs a map keyed by +;; asset). Set first in each entry point, cleared just before its (ok ...); a +;; failed assert aborts the whole call, so a reverted call never leaves it set. +(define-data-var reentrancy-locked bool false) + (define-constant SHARE-PRECISION u100000000) ;; 1e8 - matches sBTC sat precision ;; F-1 fix (v2): virtual shares + virtual assets (OpenZeppelin ERC-4626 @@ -94,17 +105,23 @@ ;; shares = amount * (total_shares + VIRTUAL-SHARES) / (pool_balance + VIRTUAL-ASSETS) (new-shares (/ (* amount (+ current-shares VIRTUAL-SHARES)) (+ pool-balance VIRTUAL-ASSETS))) ) + ;; F-9 fix: reentrancy guard, checked first. + (asserts! (not (var-get reentrancy-locked)) ERR-REENTRANT) + (var-set reentrancy-locked true) ;; F-8 fix: deposit is gated by pause, matching flash-loan and pool-v3's pv3-F3 fix. ;; withdraw is deliberately never gated, so LPs can always still exit. (asserts! (not (var-get paused)) ERR-PAUSED) (asserts! (> amount u0) ERR-ZERO-AMOUNT) + ;; Effects before interaction, matching pool-v3; if the transfer below + ;; fails the whole call reverts, these writes included. + (map-set lp-shares depositor + (+ (default-to u0 (map-get? lp-shares depositor)) new-shares)) + (var-set total-shares (+ current-shares new-shares)) (unwrap! (contract-call? 'SM3VDXK3WZZSA84XXFKAFAF15NNZX32CTSG82JFQ4.sbtc-token transfer amount depositor (as-contract tx-sender) none) ERR-TRANSFER-FAILED) - (map-set lp-shares depositor - (+ (default-to u0 (map-get? lp-shares depositor)) new-shares)) - (var-set total-shares (+ current-shares new-shares)) + (var-set reentrancy-locked false) (ok new-shares) ) ) @@ -118,6 +135,10 @@ get-balance (as-contract tx-sender)) ERR-TRANSFER-FAILED)) (sats-amount (/ (* shares (+ pool-balance VIRTUAL-ASSETS)) (+ current-shares VIRTUAL-SHARES))) ) + ;; F-9 fix: reentrancy guard. Withdraw has no callback surface of its own; + ;; this blocks a flash-loan callback from reentering withdraw (pool-v3 parity). + (asserts! (not (var-get reentrancy-locked)) ERR-REENTRANT) + (var-set reentrancy-locked true) (asserts! (> shares u0) ERR-ZERO-AMOUNT) (asserts! (>= depositor-shares shares) ERR-INSUFFICIENT-SHARES) (asserts! (> sats-amount u0) ERR-ZERO-AMOUNT) @@ -127,6 +148,7 @@ (as-contract (contract-call? 'SM3VDXK3WZZSA84XXFKAFAF15NNZX32CTSG82JFQ4.sbtc-token transfer sats-amount tx-sender withdrawer none)) ERR-TRANSFER-FAILED) + (var-set reentrancy-locked false) (ok sats-amount) ) ) @@ -148,6 +170,10 @@ get-balance (as-contract tx-sender)) ERR-REPAY-FAILED)) ) + ;; F-9 fix: reentrancy guard. A receiver reentering deposit/withdraw during + ;; the callback below now hits ERR-REENTRANT, so that call changes nothing. + (asserts! (not (var-get reentrancy-locked)) ERR-REENTRANT) + (var-set reentrancy-locked true) (asserts! (not (var-get paused)) ERR-PAUSED) (asserts! (> amount u0) ERR-ZERO-AMOUNT) (asserts! (<= amount (var-get max-single-loan)) ERR-EXCEEDS-LIMIT) @@ -173,6 +199,7 @@ (var-set total-loans (+ (var-get total-loans) u1)) (var-set total-volume (+ (var-get total-volume) amount)) (var-set total-fees (+ (var-get total-fees) (- reserve-after reserve-before))) + (var-set reentrancy-locked false) (ok true) ) ) diff --git a/contracts/flashstack-stx-pool-v3.clar b/contracts/flashstack-stx-pool-v3.clar index f4eb614..62d4ec4 100644 --- a/contracts/flashstack-stx-pool-v3.clar +++ b/contracts/flashstack-stx-pool-v3.clar @@ -5,6 +5,10 @@ ;; Here the update only PROPOSES; the new admin must call accept-admin. NOT DEPLOYED. ;; F-8 FIX -- deposit is also gated by pause now (v2's deposit was not; flash-loan was). ;; See docs/security/FINDINGS_REGISTER.md. withdraw stays ungated so LPs can always exit. +;; F-9 FIX -- reentrancy lock on deposit/withdraw/flash-loan, ported from +;; flashstack-pool-v3's pv3-F1 guard. Without it, a flash-loan receiver can +;; "repay" by calling deposit mid-callback and mint shares at the loan-depressed +;; price, diluting every LP (proven on the live v2 pools, ajv.4.8). ;; ============================================================================ ;; FlashStack STX Pool v2 -- External Liquidity Provider Model (HARDENED) ;; Anyone can deposit STX and earn yield from flash loan fees. @@ -51,6 +55,7 @@ (define-constant ERR-INVALID-FEE (err u407)) (define-constant ERR-NO-SHARES (err u408)) (define-constant ERR-INSUFFICIENT-SHARES (err u409)) +(define-constant ERR-REENTRANT (err u411)) ;; ============================================= ;; Data vars @@ -67,6 +72,12 @@ (define-data-var total-volume uint u0) (define-data-var total-fees uint u0) +;; F-9 fix: one lock shared by deposit/withdraw/flash-loan. This pool holds a +;; single asset, so a bool is the per-asset lock (pool-v3 needs a map keyed by +;; asset). Set first in each entry point, cleared just before its (ok ...); a +;; failed assert aborts the whole call, so a reverted call never leaves it set. +(define-data-var reentrancy-locked bool false) + ;; Precision multiplier for share calculations (avoids integer rounding) (define-constant SHARE-PRECISION u1000000) @@ -102,20 +113,25 @@ ;; shares = amount * (total_shares + VIRTUAL-SHARES) / (pool_balance + VIRTUAL-ASSETS) (new-shares (/ (* amount (+ current-shares VIRTUAL-SHARES)) (+ pool-balance VIRTUAL-ASSETS))) ) + ;; F-9 fix: reentrancy guard, checked first. + (asserts! (not (var-get reentrancy-locked)) ERR-REENTRANT) + (var-set reentrancy-locked true) ;; F-8 fix: deposit is gated by pause, matching flash-loan and pool-v3's pv3-F3 fix. ;; withdraw is deliberately never gated, so LPs can always still exit. (asserts! (not (var-get paused)) ERR-PAUSED) (asserts! (> amount u0) ERR-ZERO-AMOUNT) - ;; Transfer STX from depositor to pool - (unwrap! (stx-transfer? amount depositor (as-contract tx-sender)) ERR-REPAY-FAILED) - - ;; Credit shares + ;; Credit shares (effects before interaction, matching pool-v3; if the + ;; transfer below fails the whole call reverts, these writes included) (map-set lp-shares depositor (+ (default-to u0 (map-get? lp-shares depositor)) new-shares) ) (var-set total-shares (+ current-shares new-shares)) + ;; Transfer STX from depositor to pool + (unwrap! (stx-transfer? amount depositor (as-contract tx-sender)) ERR-REPAY-FAILED) + + (var-set reentrancy-locked false) (ok new-shares) ) ) @@ -131,6 +147,10 @@ ;; STX owed = shares * (pool_balance + VIRTUAL-ASSETS) / (total_shares + VIRTUAL-SHARES) (stx-amount (/ (* shares (+ pool-balance VIRTUAL-ASSETS)) (+ current-shares VIRTUAL-SHARES))) ) + ;; F-9 fix: reentrancy guard. Withdraw has no callback surface of its own; + ;; this blocks a flash-loan callback from reentering withdraw (pool-v3 parity). + (asserts! (not (var-get reentrancy-locked)) ERR-REENTRANT) + (var-set reentrancy-locked true) (asserts! (> shares u0) ERR-ZERO-AMOUNT) (asserts! (>= depositor-shares shares) ERR-INSUFFICIENT-SHARES) (asserts! (> stx-amount u0) ERR-ZERO-AMOUNT) @@ -142,6 +162,7 @@ ;; Send STX back (unwrap! (as-contract (stx-transfer? stx-amount tx-sender withdrawer)) ERR-REPAY-FAILED) + (var-set reentrancy-locked false) (ok stx-amount) ) ) @@ -160,6 +181,10 @@ (fee (if (> raw-fee u0) raw-fee u1)) (reserve-before (stx-get-balance (as-contract tx-sender))) ) + ;; F-9 fix: reentrancy guard. A receiver reentering deposit/withdraw during + ;; the callback below now hits ERR-REENTRANT, so that call changes nothing. + (asserts! (not (var-get reentrancy-locked)) ERR-REENTRANT) + (var-set reentrancy-locked true) (asserts! (not (var-get paused)) ERR-PAUSED) (asserts! (> amount u0) ERR-ZERO-AMOUNT) (asserts! (<= amount (var-get max-single-loan)) ERR-EXCEEDS-LIMIT) @@ -183,6 +208,7 @@ ;; Fee stays in pool -- automatically increases share value for all LPs + (var-set reentrancy-locked false) (ok true) ) ) diff --git a/contracts/test/flashstack-sbtc-pool-v3.clar b/contracts/test/flashstack-sbtc-pool-v3.clar index eb01dfd..0b615cc 100644 --- a/contracts/test/flashstack-sbtc-pool-v3.clar +++ b/contracts/test/flashstack-sbtc-pool-v3.clar @@ -5,6 +5,10 @@ ;; Here the update only PROPOSES; the new admin must call accept-admin. NOT DEPLOYED. ;; F-8 FIX -- deposit is also gated by pause now (v2's deposit was not; flash-loan was). ;; See docs/security/FINDINGS_REGISTER.md. withdraw stays ungated so LPs can always exit. +;; F-9 FIX -- reentrancy lock on deposit/withdraw/flash-loan, ported from +;; flashstack-pool-v3's pv3-F1 guard. Without it, a flash-loan receiver can +;; "repay" by calling deposit mid-callback and mint shares at the loan-depressed +;; price, diluting every LP (proven on the live v2 pools, ajv.4.8). ;; ============================================================================ ;; FlashStack sBTC Pool v2 (HARDENED) ;; @@ -52,6 +56,7 @@ (define-constant ERR-NO-SHARES (err u708)) (define-constant ERR-INSUFFICIENT-SHARES (err u709)) (define-constant ERR-TRANSFER-FAILED (err u710)) +(define-constant ERR-REENTRANT (err u712)) ;; ============================================= ;; State @@ -68,6 +73,12 @@ (define-data-var total-volume uint u0) (define-data-var total-fees uint u0) +;; F-9 fix: one lock shared by deposit/withdraw/flash-loan. This pool holds a +;; single asset, so a bool is the per-asset lock (pool-v3 needs a map keyed by +;; asset). Set first in each entry point, cleared just before its (ok ...); a +;; failed assert aborts the whole call, so a reverted call never leaves it set. +(define-data-var reentrancy-locked bool false) + (define-constant SHARE-PRECISION u100000000) ;; 1e8 - matches sBTC sat precision ;; F-1 fix (v2): virtual shares + virtual assets (OpenZeppelin ERC-4626 @@ -94,17 +105,23 @@ ;; shares = amount * (total_shares + VIRTUAL-SHARES) / (pool_balance + VIRTUAL-ASSETS) (new-shares (/ (* amount (+ current-shares VIRTUAL-SHARES)) (+ pool-balance VIRTUAL-ASSETS))) ) + ;; F-9 fix: reentrancy guard, checked first. + (asserts! (not (var-get reentrancy-locked)) ERR-REENTRANT) + (var-set reentrancy-locked true) ;; F-8 fix: deposit is gated by pause, matching flash-loan and pool-v3's pv3-F3 fix. ;; withdraw is deliberately never gated, so LPs can always still exit. (asserts! (not (var-get paused)) ERR-PAUSED) (asserts! (> amount u0) ERR-ZERO-AMOUNT) + ;; Effects before interaction, matching pool-v3; if the transfer below + ;; fails the whole call reverts, these writes included. + (map-set lp-shares depositor + (+ (default-to u0 (map-get? lp-shares depositor)) new-shares)) + (var-set total-shares (+ current-shares new-shares)) (unwrap! (contract-call? .sbtc-token transfer amount depositor (as-contract tx-sender) none) ERR-TRANSFER-FAILED) - (map-set lp-shares depositor - (+ (default-to u0 (map-get? lp-shares depositor)) new-shares)) - (var-set total-shares (+ current-shares new-shares)) + (var-set reentrancy-locked false) (ok new-shares) ) ) @@ -118,6 +135,10 @@ get-balance (as-contract tx-sender)) ERR-TRANSFER-FAILED)) (sats-amount (/ (* shares (+ pool-balance VIRTUAL-ASSETS)) (+ current-shares VIRTUAL-SHARES))) ) + ;; F-9 fix: reentrancy guard. Withdraw has no callback surface of its own; + ;; this blocks a flash-loan callback from reentering withdraw (pool-v3 parity). + (asserts! (not (var-get reentrancy-locked)) ERR-REENTRANT) + (var-set reentrancy-locked true) (asserts! (> shares u0) ERR-ZERO-AMOUNT) (asserts! (>= depositor-shares shares) ERR-INSUFFICIENT-SHARES) (asserts! (> sats-amount u0) ERR-ZERO-AMOUNT) @@ -127,6 +148,7 @@ (as-contract (contract-call? .sbtc-token transfer sats-amount tx-sender withdrawer none)) ERR-TRANSFER-FAILED) + (var-set reentrancy-locked false) (ok sats-amount) ) ) @@ -148,6 +170,10 @@ get-balance (as-contract tx-sender)) ERR-REPAY-FAILED)) ) + ;; F-9 fix: reentrancy guard. A receiver reentering deposit/withdraw during + ;; the callback below now hits ERR-REENTRANT, so that call changes nothing. + (asserts! (not (var-get reentrancy-locked)) ERR-REENTRANT) + (var-set reentrancy-locked true) (asserts! (not (var-get paused)) ERR-PAUSED) (asserts! (> amount u0) ERR-ZERO-AMOUNT) (asserts! (<= amount (var-get max-single-loan)) ERR-EXCEEDS-LIMIT) @@ -173,6 +199,7 @@ (var-set total-loans (+ (var-get total-loans) u1)) (var-set total-volume (+ (var-get total-volume) amount)) (var-set total-fees (+ (var-get total-fees) (- reserve-after reserve-before))) + (var-set reentrancy-locked false) (ok true) ) ) diff --git a/contracts/test/flashstack-stx-pool-v3.clar b/contracts/test/flashstack-stx-pool-v3.clar index 90f6595..6366659 100644 --- a/contracts/test/flashstack-stx-pool-v3.clar +++ b/contracts/test/flashstack-stx-pool-v3.clar @@ -5,6 +5,10 @@ ;; Here the update only PROPOSES; the new admin must call accept-admin. NOT DEPLOYED. ;; F-8 FIX -- deposit is also gated by pause now (v2's deposit was not; flash-loan was). ;; See docs/security/FINDINGS_REGISTER.md. withdraw stays ungated so LPs can always exit. +;; F-9 FIX -- reentrancy lock on deposit/withdraw/flash-loan, ported from +;; flashstack-pool-v3's pv3-F1 guard. Without it, a flash-loan receiver can +;; "repay" by calling deposit mid-callback and mint shares at the loan-depressed +;; price, diluting every LP (proven on the live v2 pools, ajv.4.8). ;; ============================================================================ ;; FlashStack STX Pool v2 -- External Liquidity Provider Model (HARDENED) ;; Anyone can deposit STX and earn yield from flash loan fees. @@ -51,6 +55,7 @@ (define-constant ERR-INVALID-FEE (err u407)) (define-constant ERR-NO-SHARES (err u408)) (define-constant ERR-INSUFFICIENT-SHARES (err u409)) +(define-constant ERR-REENTRANT (err u411)) ;; ============================================= ;; Data vars @@ -67,6 +72,12 @@ (define-data-var total-volume uint u0) (define-data-var total-fees uint u0) +;; F-9 fix: one lock shared by deposit/withdraw/flash-loan. This pool holds a +;; single asset, so a bool is the per-asset lock (pool-v3 needs a map keyed by +;; asset). Set first in each entry point, cleared just before its (ok ...); a +;; failed assert aborts the whole call, so a reverted call never leaves it set. +(define-data-var reentrancy-locked bool false) + ;; Precision multiplier for share calculations (avoids integer rounding) (define-constant SHARE-PRECISION u1000000) @@ -102,20 +113,25 @@ ;; shares = amount * (total_shares + VIRTUAL-SHARES) / (pool_balance + VIRTUAL-ASSETS) (new-shares (/ (* amount (+ current-shares VIRTUAL-SHARES)) (+ pool-balance VIRTUAL-ASSETS))) ) + ;; F-9 fix: reentrancy guard, checked first. + (asserts! (not (var-get reentrancy-locked)) ERR-REENTRANT) + (var-set reentrancy-locked true) ;; F-8 fix: deposit is gated by pause, matching flash-loan and pool-v3's pv3-F3 fix. ;; withdraw is deliberately never gated, so LPs can always still exit. (asserts! (not (var-get paused)) ERR-PAUSED) (asserts! (> amount u0) ERR-ZERO-AMOUNT) - ;; Transfer STX from depositor to pool - (unwrap! (stx-transfer? amount depositor (as-contract tx-sender)) ERR-REPAY-FAILED) - - ;; Credit shares + ;; Credit shares (effects before interaction, matching pool-v3; if the + ;; transfer below fails the whole call reverts, these writes included) (map-set lp-shares depositor (+ (default-to u0 (map-get? lp-shares depositor)) new-shares) ) (var-set total-shares (+ current-shares new-shares)) + ;; Transfer STX from depositor to pool + (unwrap! (stx-transfer? amount depositor (as-contract tx-sender)) ERR-REPAY-FAILED) + + (var-set reentrancy-locked false) (ok new-shares) ) ) @@ -131,6 +147,10 @@ ;; STX owed = shares * (pool_balance + VIRTUAL-ASSETS) / (total_shares + VIRTUAL-SHARES) (stx-amount (/ (* shares (+ pool-balance VIRTUAL-ASSETS)) (+ current-shares VIRTUAL-SHARES))) ) + ;; F-9 fix: reentrancy guard. Withdraw has no callback surface of its own; + ;; this blocks a flash-loan callback from reentering withdraw (pool-v3 parity). + (asserts! (not (var-get reentrancy-locked)) ERR-REENTRANT) + (var-set reentrancy-locked true) (asserts! (> shares u0) ERR-ZERO-AMOUNT) (asserts! (>= depositor-shares shares) ERR-INSUFFICIENT-SHARES) (asserts! (> stx-amount u0) ERR-ZERO-AMOUNT) @@ -142,6 +162,7 @@ ;; Send STX back (unwrap! (as-contract (stx-transfer? stx-amount tx-sender withdrawer)) ERR-REPAY-FAILED) + (var-set reentrancy-locked false) (ok stx-amount) ) ) @@ -160,6 +181,10 @@ (fee (if (> raw-fee u0) raw-fee u1)) (reserve-before (stx-get-balance (as-contract tx-sender))) ) + ;; F-9 fix: reentrancy guard. A receiver reentering deposit/withdraw during + ;; the callback below now hits ERR-REENTRANT, so that call changes nothing. + (asserts! (not (var-get reentrancy-locked)) ERR-REENTRANT) + (var-set reentrancy-locked true) (asserts! (not (var-get paused)) ERR-PAUSED) (asserts! (> amount u0) ERR-ZERO-AMOUNT) (asserts! (<= amount (var-get max-single-loan)) ERR-EXCEEDS-LIMIT) @@ -183,6 +208,7 @@ ;; Fee stays in pool -- automatically increases share value for all LPs + (var-set reentrancy-locked false) (ok true) ) ) diff --git a/contracts/test/test-sbtc-pool-v3-receiver-reentrant.clar b/contracts/test/test-sbtc-pool-v3-receiver-reentrant.clar new file mode 100644 index 0000000..1b04443 --- /dev/null +++ b/contracts/test/test-sbtc-pool-v3-receiver-reentrant.clar @@ -0,0 +1,50 @@ +;; TEST-ONLY receiver for flashstack-sbtc-pool-v3's F-9 fix (SIP-010 sBTC). One contract, five +;; callback behaviours selected by set-mode, so the regression suite can drive +;; every lock path through the same approved receiver: +;; u0 honest: repay amount + fee by plain transfer +;; u1 F-9 vector: "repay" by calling the pool's own deposit (amount + fee) +;; u2 reenter withdraw on shares seeded earlier by seed-deposit, then repay +;; u3 under-repay: return ok without repaying (ERR-REPAY-FAILED path) +;; u4 reenter flash-loan itself (via the honest test-sbtc-pool-receiver-good, +;; since a contract cannot pass itself as a trait), then repay. The VM +;; aborts this with CircularReference before the pool's lock is reached. +;; Pool calls are wrapped in as-contract so they are funded by, and credited +;; to, this contract (see test-pool-v2-receiver-deposit-reentrant.clar), and +;; use try! so a blocked reentry surfaces the pool's own ERR-REENTRANT. +(impl-trait .sbtc-flash-receiver-trait.sbtc-flash-receiver-trait) + +(define-data-var mode uint u0) +(define-data-var seeded-shares uint u0) + +(define-public (set-mode (m uint)) + (ok (var-set mode m)) +) + +(define-public (seed-deposit (amount uint)) + (let ((s (try! (as-contract (contract-call? .flashstack-sbtc-pool-v3 deposit amount))))) + (ok (var-set seeded-shares s)) + ) +) + +(define-public (execute-sbtc-flash (amount uint) (core principal)) + (let ( + (fee-bp (unwrap! (contract-call? .flashstack-sbtc-pool-v3 get-fee-basis-points) (err u901))) + (raw-fee (/ (* amount fee-bp) u10000)) + (fee (if (> raw-fee u0) raw-fee u1)) + (m (var-get mode)) + ) + (if (is-eq m u1) + (try! (as-contract (contract-call? .flashstack-sbtc-pool-v3 deposit (+ amount fee)))) + u0) + (if (is-eq m u2) + (try! (as-contract (contract-call? .flashstack-sbtc-pool-v3 withdraw (var-get seeded-shares)))) + u0) + (if (is-eq m u4) + (try! (contract-call? .flashstack-sbtc-pool-v3 flash-loan u1000 .test-sbtc-pool-receiver-good)) + false) + (if (or (is-eq m u0) (is-eq m u2) (is-eq m u4)) + (try! (as-contract (contract-call? .sbtc-token transfer (+ amount fee) tx-sender core none))) + false) + (ok true) + ) +) diff --git a/contracts/test/test-stx-pool-v3-receiver-reentrant.clar b/contracts/test/test-stx-pool-v3-receiver-reentrant.clar new file mode 100644 index 0000000..e3de100 --- /dev/null +++ b/contracts/test/test-stx-pool-v3-receiver-reentrant.clar @@ -0,0 +1,50 @@ +;; TEST-ONLY receiver for flashstack-stx-pool-v3's F-9 fix (native STX). One contract, five +;; callback behaviours selected by set-mode, so the regression suite can drive +;; every lock path through the same approved receiver: +;; u0 honest: repay amount + fee by plain transfer +;; u1 F-9 vector: "repay" by calling the pool's own deposit (amount + fee) +;; u2 reenter withdraw on shares seeded earlier by seed-deposit, then repay +;; u3 under-repay: return ok without repaying (ERR-REPAY-FAILED path) +;; u4 reenter flash-loan itself (via the honest test-pool-receiver-good, +;; since a contract cannot pass itself as a trait), then repay. The VM +;; aborts this with CircularReference before the pool's lock is reached. +;; Pool calls are wrapped in as-contract so they are funded by, and credited +;; to, this contract (see test-pool-v2-receiver-deposit-reentrant.clar), and +;; use try! so a blocked reentry surfaces the pool's own ERR-REENTRANT. +(impl-trait .stx-flash-receiver-trait.stx-flash-receiver-trait) + +(define-data-var mode uint u0) +(define-data-var seeded-shares uint u0) + +(define-public (set-mode (m uint)) + (ok (var-set mode m)) +) + +(define-public (seed-deposit (amount uint)) + (let ((s (try! (as-contract (contract-call? .flashstack-stx-pool-v3 deposit amount))))) + (ok (var-set seeded-shares s)) + ) +) + +(define-public (execute-stx-flash (amount uint) (core principal)) + (let ( + (fee-bp (unwrap! (contract-call? .flashstack-stx-pool-v3 get-fee-basis-points) (err u901))) + (raw-fee (/ (* amount fee-bp) u10000)) + (fee (if (> raw-fee u0) raw-fee u1)) + (m (var-get mode)) + ) + (if (is-eq m u1) + (try! (as-contract (contract-call? .flashstack-stx-pool-v3 deposit (+ amount fee)))) + u0) + (if (is-eq m u2) + (try! (as-contract (contract-call? .flashstack-stx-pool-v3 withdraw (var-get seeded-shares)))) + u0) + (if (is-eq m u4) + (try! (contract-call? .flashstack-stx-pool-v3 flash-loan u1000 .test-pool-receiver-good)) + false) + (if (or (is-eq m u0) (is-eq m u2) (is-eq m u4)) + (try! (as-contract (stx-transfer? (+ amount fee) tx-sender core))) + false) + (ok true) + ) +) diff --git a/docs/security/FINDINGS_REGISTER.md b/docs/security/FINDINGS_REGISTER.md index ea2bb62..2fc1d63 100644 --- a/docs/security/FINDINGS_REGISTER.md +++ b/docs/security/FINDINGS_REGISTER.md @@ -34,7 +34,7 @@ admin action (approving a receiver), not a privileged one — see its row.** | pv3-F1 | Medium | `flashstack-pool-v3` | A receiver reentering `deposit()` for the same asset during its own flash-loan callback got its real deposit inflow miscounted as fee revenue | **Fixed** — per-asset reentrancy lock (deliberately not global, to preserve legitimate cross-asset flash-loan flows). Found by an external reviewer, independently reproduced before fixing. | | pv3-F2 | Medium | `flashstack-pool-v3` | Flat virtual-shares constant, not calibrated per asset decimals | **Fixed** — `share-scale` computed from a live `get-decimals()` call at listing time, not a hardcoded table. Proven directly (sBTC → 1e8, USDCx → 1e6). | | pv3-F3 | Medium | `flashstack-pool-v3` | `deposit` was not gated by pause (oversight — `flash-loan` was) | **Fixed** — `deposit` now asserts both global and per-asset pause, matching `flash-loan`. | -| **F-9** | **Critical** | `flashstack-stx-pool-v2` (**CONFIRMED, live, un-mitigated**); `flashstack-sbtc-pool-v2` (**CONFIRMED by a passing simnet proof, 2026-10-04**) | **The same bug class as pv3-F1 (reentrant deposit during a flash-loan callback), on the two pools pv3-F1's own fix was never ported to.** Neither pool has a reentrancy lock. A flash-loan receiver can repay by calling the pool's own `deposit` instead of a plain transfer. That satisfies the balance-delta repayment check (the pool's balance genuinely increases by `amount + fee`) but **also mints the caller LP shares**, priced against the pool's balance as depressed by this same loan's outbound transfer — a materially cheaper share price than any honest depositor gets, which dilutes every existing LP. Shares are credited to whichever principal is `tx-sender` for the nested `deposit` call: the receiver contract, if it wraps that call in `as-contract` so the deposit is funded out of the loan it is already holding — the construction that keeps the attack fee-sized. Without `as-contract`, `deposit`'s transfer is instead sourced from the attacker's own wallet, which only looks cheap in a test harness that gives every account a large default balance; in practice that path requires fronting nearly the entire loan amount and isn't a fee-sized attack at all. | **CONFIRMED by a passing simnet proof** (`tests/stx-pool-v2-deposit-reentrancy.test.ts`, receiver `contracts/test/test-pool-v2-receiver-deposit-reentrant.clar`), against a `contracts/test/flashstack-stx-pool-v2.clar` copy that is a faithful localization of the **live, funds-bearing** mainnet contract (`SPR9PQAN…`, 1.000000 STX, `paused=false` as of the CONTRACT_INVENTORY snapshot). Measured in one transaction, with the attacker fronting only fee-sized capital (50,000 µSTX — the real minimum that covers the 0.05% fee on a 99,000,000 µSTX / 99 STX loan; below that the self-deposit fails for insufficient balance) via the `as-contract` construction: the receiver contract ends up with 9,904,940,194,109,305 of 10,004,940,194,109,305 total shares (effectively all of them) worth 99,049,499 µSTX, a ~1,981x return on the attacker's 50,000 µSTX outlay (~2,001x at the exact minimum of 49,500 µSTX — one µSTX less and the self-deposit fails), with the attacker's own wallet spending nothing further once that buffer is in place; the sole honest LP's 100,000,000 µSTX deposit is left worth 1,000,000 µSTX — a 99% loss, extracted in a single transaction. (*Corrected 2026-10-01 — independently re-derived by Matt Glory, whose own repro used `as-contract` and caught that an earlier version of this PoC, without it, had credited shares to the attacker's EOA and silently drawn the deposit from the attacker's own wallet, overstating the return as ~9,905x on a 10,000 µSTX outlay. Re-verified a second time via a from-scratch hand derivation of the share-mint formula plus a fresh on-chain-equivalent read-only query, cross-checked against each other and against a boundary test (49,499 µSTX fails, 49,500 succeeds): this also caught that the share-count figures themselves (previously ending "…109,304 of …209,304") were off by JS `Number()` precision loss on a value exceeding `Number.MAX_SAFE_INTEGER` — corrected here to the exact on-chain uint values. The 99% LP-loss and 99,049,499 µSTX value figures were unaffected by either correction.*) `flashstack-sbtc-pool-v2` — **CONFIRMED by a passing simnet proof** (`tests/sbtc-pool-v2-deposit-reentrancy.test.ts`, receiver `contracts/test/test-sbtc-pool-v2-receiver-deposit-reentrant.clar`), 2026-10-04: independently reproduced through the pool's SIP-010 repayment path (a `contract-call?` to `sbtc-token`'s `transfer`, which asserts `tx-sender == sender`) rather than assumed to carry over from the STX proof's `stx-transfer?` path. Measured with a 10,000,000-sat (0.1 BTC) honest LP deposit, a 9,900,000-sat loan (99% of reserve, under the pool's own max-single-loan cap), and a 5,000-sat attacker buffer (covers the 4,950-sat fee): the receiver contract ends up holding shares funded entirely out of the loan proceeds — the attacker's own wallet spends nothing beyond the pre-funded buffer — while the honest LP's value drops below their original deposit. A control receiver that repays by plain transfer instead of `deposit` gets exactly zero shares, isolating the effect to the deposit-reentrancy mechanism and ruling out a harness artifact. Same vector, same exposure, 44,990 sats live. The cores (`flashstack-stx-core`, `flashstack-sbtc-core`) are **not** exposed: their equivalent of `deposit` (`deposit-reserve`) is `tx-sender == admin`-gated, which a receiver callback cannot satisfy since `tx-sender` is the original caller throughout the call chain, not the receiver contract or the admin. | **Open, but CONFIRMED DORMANT as of 2026-10-01** — independently re-verified against live chain state, not assumed: (1) `contracts/test/flashstack-stx-pool-v2.clar` (what the PoC ran against) diffs byte-for-byte against a fresh fetch of the deployed source at `SPR9PQAN…`, differing only in the trait address literal (the expected localization) — the PoC is against the real live contract, not a drifted copy; (2) both live pools read `paused=false`, `total-loans=0` right now via direct read-only calls; (3) the admin wallet's **complete** transaction history (24/24 txs, not a sample) contains **zero** `add-approved-receiver` calls against either `flashstack-stx-pool-v2` or `flashstack-sbtc-pool-v2` — every `add-approved-receiver` call it has ever made targets `flashstack-stx-core` instead. **No receiver has ever been whitelisted on either v2 pool, so the vector is unreachable today** — not because of any mitigation, but because nothing has opened the door yet. A control run (same harness, `test-pool-receiver-good` repaying by plain transfer) credits the caller zero shares, isolating the effect to the deposit-reentrancy specifically, not a harness artifact. **The real deadline is the first `add-approved-receiver` call on either pool** — nothing in the current review process would catch this before that call, since "repays via deposit" and "repays via transfer" both make `flash-loan` return `(ok true)`. **Zero-code mitigation available today: `set-paused true` on both live v2 pools blocks `flash-loan` entirely (confirmed in both contracts' source) — `deposit`/`withdraw` correctly unaffected — though with no receiver ever approved, pausing is precautionary rather than stopping an in-progress loss.** No fix is possible in place; these are immutable deployed contracts. A real fix mirrors pv3-F1's per-asset(-equivalent) lock and would need a v3-equivalent successor for these two pools, the same posture already adopted for BC1. Found by the Security & Contract Lead, 2026-10-01, while writing the threat model (`Flashstack-ajv.1.5`). Tracked in `Flashstack-ajv.4.8`. | +| **F-9** | **Critical** | `flashstack-stx-pool-v2` (**CONFIRMED, live, un-mitigated**); `flashstack-sbtc-pool-v2` (**CONFIRMED by a passing simnet proof, 2026-10-04**) | **The same bug class as pv3-F1 (reentrant deposit during a flash-loan callback), on the two pools pv3-F1's own fix was never ported to.** Neither pool has a reentrancy lock. A flash-loan receiver can repay by calling the pool's own `deposit` instead of a plain transfer. That satisfies the balance-delta repayment check (the pool's balance genuinely increases by `amount + fee`) but **also mints the caller LP shares**, priced against the pool's balance as depressed by this same loan's outbound transfer — a materially cheaper share price than any honest depositor gets, which dilutes every existing LP. Shares are credited to whichever principal is `tx-sender` for the nested `deposit` call: the receiver contract, if it wraps that call in `as-contract` so the deposit is funded out of the loan it is already holding — the construction that keeps the attack fee-sized. Without `as-contract`, `deposit`'s transfer is instead sourced from the attacker's own wallet, which only looks cheap in a test harness that gives every account a large default balance; in practice that path requires fronting nearly the entire loan amount and isn't a fee-sized attack at all. | **CONFIRMED by a passing simnet proof** (`tests/stx-pool-v2-deposit-reentrancy.test.ts`, receiver `contracts/test/test-pool-v2-receiver-deposit-reentrant.clar`), against a `contracts/test/flashstack-stx-pool-v2.clar` copy that is a faithful localization of the **live, funds-bearing** mainnet contract (`SPR9PQAN…`, 1.000000 STX, `paused=false` as of the CONTRACT_INVENTORY snapshot). Measured in one transaction, with the attacker fronting only fee-sized capital (50,000 µSTX — the real minimum that covers the 0.05% fee on a 99,000,000 µSTX / 99 STX loan; below that the self-deposit fails for insufficient balance) via the `as-contract` construction: the receiver contract ends up with 9,904,940,194,109,305 of 10,004,940,194,109,305 total shares (effectively all of them) worth 99,049,499 µSTX, a ~1,981x return on the attacker's 50,000 µSTX outlay (~2,001x at the exact minimum of 49,500 µSTX — one µSTX less and the self-deposit fails), with the attacker's own wallet spending nothing further once that buffer is in place; the sole honest LP's 100,000,000 µSTX deposit is left worth 1,000,000 µSTX — a 99% loss, extracted in a single transaction. (*Corrected 2026-10-01 — independently re-derived by Matt Glory, whose own repro used `as-contract` and caught that an earlier version of this PoC, without it, had credited shares to the attacker's EOA and silently drawn the deposit from the attacker's own wallet, overstating the return as ~9,905x on a 10,000 µSTX outlay. Re-verified a second time via a from-scratch hand derivation of the share-mint formula plus a fresh on-chain-equivalent read-only query, cross-checked against each other and against a boundary test (49,499 µSTX fails, 49,500 succeeds): this also caught that the share-count figures themselves (previously ending "…109,304 of …209,304") were off by JS `Number()` precision loss on a value exceeding `Number.MAX_SAFE_INTEGER` — corrected here to the exact on-chain uint values. The 99% LP-loss and 99,049,499 µSTX value figures were unaffected by either correction.*) `flashstack-sbtc-pool-v2` — **CONFIRMED by a passing simnet proof** (`tests/sbtc-pool-v2-deposit-reentrancy.test.ts`, receiver `contracts/test/test-sbtc-pool-v2-receiver-deposit-reentrant.clar`), 2026-10-04: independently reproduced through the pool's SIP-010 repayment path (a `contract-call?` to `sbtc-token`'s `transfer`, which asserts `tx-sender == sender`) rather than assumed to carry over from the STX proof's `stx-transfer?` path. Measured with a 10,000,000-sat (0.1 BTC) honest LP deposit, a 9,900,000-sat loan (99% of reserve, under the pool's own max-single-loan cap), and a 5,000-sat attacker buffer (covers the 4,950-sat fee): the receiver contract ends up holding shares funded entirely out of the loan proceeds — the attacker's own wallet spends nothing beyond the pre-funded buffer — while the honest LP's value drops below their original deposit. A control receiver that repays by plain transfer instead of `deposit` gets exactly zero shares, isolating the effect to the deposit-reentrancy mechanism and ruling out a harness artifact. Same vector, same exposure, 44,990 sats live. The cores (`flashstack-stx-core`, `flashstack-sbtc-core`) are **not** exposed: their equivalent of `deposit` (`deposit-reserve`) is `tx-sender == admin`-gated, which a receiver callback cannot satisfy since `tx-sender` is the original caller throughout the call chain, not the receiver contract or the admin. | **Open, but CONFIRMED DORMANT as of 2026-10-01** — independently re-verified against live chain state, not assumed: (1) `contracts/test/flashstack-stx-pool-v2.clar` (what the PoC ran against) diffs byte-for-byte against a fresh fetch of the deployed source at `SPR9PQAN…`, differing only in the trait address literal (the expected localization) — the PoC is against the real live contract, not a drifted copy; (2) both live pools read `paused=false`, `total-loans=0` right now via direct read-only calls; (3) the admin wallet's **complete** transaction history (24/24 txs, not a sample) contains **zero** `add-approved-receiver` calls against either `flashstack-stx-pool-v2` or `flashstack-sbtc-pool-v2` — every `add-approved-receiver` call it has ever made targets `flashstack-stx-core` instead. **No receiver has ever been whitelisted on either v2 pool, so the vector is unreachable today** — not because of any mitigation, but because nothing has opened the door yet. A control run (same harness, `test-pool-receiver-good` repaying by plain transfer) credits the caller zero shares, isolating the effect to the deposit-reentrancy specifically, not a harness artifact. **The real deadline is the first `add-approved-receiver` call on either pool** — nothing in the current review process would catch this before that call, since "repays via deposit" and "repays via transfer" both make `flash-loan` return `(ok true)`. **Zero-code mitigation available today: `set-paused true` on both live v2 pools blocks `flash-loan` entirely (confirmed in both contracts' source) — `deposit`/`withdraw` correctly unaffected — though with no receiver ever approved, pausing is precautionary rather than stopping an in-progress loss.** No fix is possible in place; these are immutable deployed contracts. A real fix mirrors pv3-F1's per-asset(-equivalent) lock and would need a v3-equivalent successor for these two pools, the same posture already adopted for BC1. **Fixed in the repository only (2026-10-05, `Flashstack-ajv.4.9`) — not a mainnet remediation for any deployed contract.** The successors `flashstack-stx-pool-v3` / `flashstack-sbtc-pool-v3` turned out to ship without the lock too (PR #84); both now carry pool-v3's pv3-F1 guard as a single `reentrancy-locked` bool shared by `deposit`, `withdraw` and `flash-loan` (withdraw included for parity with pool-v3, Matt's decision), returning `ERR-REENTRANT` (`u411` / `u712`). Proven by `tests/v3-pools-reentrancy-lock.test.ts` against both pools separately: deposit- and withdraw-reentry from the flash-loan callback are rejected with zero state change, and the lock never stays set after a success, a blocked reentry, or an under-repay revert; the lock-dependent tests (4 per pool) fail against the pre-fix source. A nested `flash-loan` on the same pool never reaches the lock: the Clarity VM aborts it as a `CircularReference`, with or without this fix (pinned by the same file). Both successors remain **undeployed**, and the live v2 pools are immutable and unchanged by this; deploying the successors requires the separate approved process (receiver vetting, testnet plan, mainnet sign-off) and the v2 disposition (`Flashstack-ajv.4.4`). Found by the Security & Contract Lead, 2026-10-01, while writing the threat model (`Flashstack-ajv.1.5`). Tracked in `Flashstack-ajv.4.8`. | --- diff --git a/tests/mainnet-plan-guard.test.ts b/tests/mainnet-plan-guard.test.ts index 34ac280..576db65 100644 --- a/tests/mainnet-plan-guard.test.ts +++ b/tests/mainnet-plan-guard.test.ts @@ -62,8 +62,10 @@ const KNOWN_TEST_PATHS = [ "contracts/test/test-receiver-good.clar", "contracts/test/test-sbtc-pool-receiver-good.clar", "contracts/test/test-sbtc-pool-v2-receiver-deposit-reentrant.clar", + "contracts/test/test-sbtc-pool-v3-receiver-reentrant.clar", "contracts/test/test-sbtc-receiver-bad.clar", "contracts/test/test-sbtc-receiver-good.clar", + "contracts/test/test-stx-pool-v3-receiver-reentrant.clar", ]; const CLARINET = process.env.CLARINET_BIN || "clarinet"; diff --git a/tests/v3-pools-reentrancy-lock.test.ts b/tests/v3-pools-reentrancy-lock.test.ts new file mode 100644 index 0000000..b5f2f02 --- /dev/null +++ b/tests/v3-pools-reentrancy-lock.test.ts @@ -0,0 +1,147 @@ +import { describe, expect, it, beforeEach } from "vitest"; +import { Cl } from "@stacks/transactions"; + +/** + * F-9 regression for the undeployed v3 successors (Flashstack-ajv.4.9). + * + * The live v2 pools let a flash-loan receiver "repay" by calling the pool's own + * deposit mid-callback, minting shares at the loan-depressed price (proven in + * stx-/sbtc-pool-v2-deposit-reentrancy.test.ts). flashstack-stx-pool-v3 and + * flashstack-sbtc-pool-v3 shipped without a fix (PR #84). They now carry the + * pool-v3 pv3-F1 lock: one bool shared by deposit, withdraw and flash-loan + * (withdraw included per Matt's Option B decision, 2026-10-05). + * + * Pinned, for both pools separately (STX uses stx-transfer?, sBTC a SIP-010 + * transfer -- do not assume one proves the other): + * - the happy paths still work (the lock must not over-block) + * - flash-loan -> deposit is rejected (the F-9 vector itself) + * - flash-loan -> withdraw is rejected (Option B) + * - flash-loan -> flash-loan aborts (the Clarity VM itself rejects the + * nested call, before the lock is reached) + * - a rejected attempt mutates nothing (not just "returned err") + * - the lock never stays set (after success, a blocked reentry, + * or an under-repay revert) + */ + +const POOLS = [ + { asset: "stx", pool: "flashstack-stx-pool-v3", rx: "test-stx-pool-v3-receiver-reentrant", nestedRx: "test-pool-receiver-good", errReentrant: 411, errRepay: 402 }, + { asset: "sbtc", pool: "flashstack-sbtc-pool-v3", rx: "test-sbtc-pool-v3-receiver-reentrant", nestedRx: "test-sbtc-pool-receiver-good", errReentrant: 712, errRepay: 702 }, +] as const; + +const MODE = { honest: 0, deposit: 1, withdraw: 2, underRepay: 3, nestedLoan: 4 } as const; + +const LP_DEP = 10_000_000; // the honest LP's deposit +const LOAN = 9_900_000; // 99% of the reserve, under both pools' max-single-loan +const FEE = 4_950; // 0.05% of LOAN +const RX_FUND = 2_000_000; // receiver's own capital: covers the fee and a seed deposit +const RX_SEED = 1_000_000; // shares the receiver holds before the withdraw-reentry attempt + +for (const { asset, pool, rx, nestedRx, errReentrant, errRepay } of POOLS) { + describe(`F-9: ${pool} reentrancy lock`, () => { + let deployer: string, attacker: string, lp: string, rxPrincipal: string; + + const fund = (amount: number, to: string) => + asset === "sbtc" + ? simnet.callPublicFn("sbtc-token", "mint", [Cl.uint(amount), Cl.principal(to)], deployer) + : simnet.transferSTX(amount, to, deployer); + const setMode = (m: number) => simnet.callPublicFn(rx, "set-mode", [Cl.uint(m)], deployer); + const flashLoan = () => + simnet.callPublicFn(pool, "flash-loan", [Cl.uint(LOAN), Cl.contractPrincipal(deployer, rx)], attacker).result; + const shares = (who: string) => + Number((simnet.callReadOnlyFn(pool, "get-shares", [Cl.principal(who)], deployer).result as any).value); + // Everything an attempt could corrupt: pool balance, total-shares, loan/fee + // stats, and both share holders' positions. + const snapshot = () => ({ + stats: simnet.callReadOnlyFn(pool, "get-stats", [], deployer).result, + lp: shares(lp), + rx: shares(rxPrincipal), + }); + + beforeEach(() => { + deployer = simnet.getAccounts().get("deployer")!; + attacker = simnet.getAccounts().get("wallet_1")!; + lp = simnet.getAccounts().get("wallet_2")!; + rxPrincipal = `${deployer}.${rx}`; + + if (asset === "sbtc") fund(LP_DEP, lp); + // First deposit in a fresh simnet: also proves the lock starts unset. + expect(simnet.callPublicFn(pool, "deposit", [Cl.uint(LP_DEP)], lp).result.type).toBe("ok"); + simnet.callPublicFn(pool, "add-approved-receiver", [Cl.principal(rxPrincipal)], deployer); + fund(RX_FUND, rxPrincipal); + }); + + it("happy path: honest flash-loan, deposit and withdraw all still succeed in sequence", () => { + setMode(MODE.honest); + expect(flashLoan()).toBeOk(Cl.bool(true)); + if (asset === "sbtc") fund(LP_DEP, lp); + const minted = simnet.callPublicFn(pool, "deposit", [Cl.uint(LP_DEP)], lp).result as any; + expect(minted.type).toBe("ok"); + expect(simnet.callPublicFn(pool, "withdraw", [minted.value], lp).result.type).toBe("ok"); + expect(flashLoan()).toBeOk(Cl.bool(true)); + + const stats = (simnet.callReadOnlyFn(pool, "get-stats", [], deployer).result as any).value.value; + expect(stats["total-loans"]).toBeUint(2); + expect(stats["total-fees"]).toBeUint(2 * FEE); + }); + + it("flash-loan -> callback -> deposit (the F-9 vector) is rejected with ERR-REENTRANT", () => { + setMode(MODE.deposit); + expect(flashLoan()).toBeErr(Cl.uint(errReentrant)); + }); + + it("the rejected deposit-reentry mutates nothing: no shares minted, no stats, no balance change", () => { + const before = snapshot(); + setMode(MODE.deposit); + flashLoan(); + expect(snapshot()).toEqual(before); + expect(shares(rxPrincipal)).toBe(0); + }); + + it("flash-loan -> callback -> withdraw (Option B) is rejected, receiver's existing shares untouched", () => { + expect(simnet.callPublicFn(rx, "seed-deposit", [Cl.uint(RX_SEED)], deployer).result.type).toBe("ok"); + const before = snapshot(); + expect(before.rx).toBeGreaterThan(0); + + setMode(MODE.withdraw); + expect(flashLoan()).toBeErr(Cl.uint(errReentrant)); + expect(snapshot()).toEqual(before); + }); + + it("flash-loan -> callback -> flash-loan (nested, same pool) aborts in the VM, not via the lock", () => { + // The nested loan goes through an honest, approved, funded receiver. It + // never reaches the lock: Clarity refuses to re-enter a function that is + // already on the call stack (RuntimeCheck CircularReference), so the + // whole transaction aborts. Same result with or without the F-9 lock -- + // pinned here so the claim that this path is unreachable stays checked. + const nested = `${deployer}.${nestedRx}`; + simnet.callPublicFn(pool, "add-approved-receiver", [Cl.principal(nested)], deployer); + fund(10_000, nested); + const before = snapshot(); + + setMode(MODE.nestedLoan); + expect(() => flashLoan()).toThrow(/CircularReference/); + expect(snapshot()).toEqual(before); + }); + + it("lock is released after a blocked reentry: an honest flash-loan succeeds right after", () => { + setMode(MODE.deposit); + expect(flashLoan()).toBeErr(Cl.uint(errReentrant)); + setMode(MODE.honest); + expect(flashLoan()).toBeOk(Cl.bool(true)); + }); + + it("lock is released after an under-repay revert (fails after the callback returns ok)", () => { + const before = snapshot(); + setMode(MODE.underRepay); + expect(flashLoan()).toBeErr(Cl.uint(errRepay)); + expect(snapshot()).toEqual(before); + + // Every entry point that takes the lock still works afterward. + expect(simnet.callPublicFn(pool, "withdraw", [Cl.uint(Math.floor(shares(lp) / 2))], lp).result.type).toBe("ok"); + if (asset === "sbtc") fund(LP_DEP, lp); + expect(simnet.callPublicFn(pool, "deposit", [Cl.uint(LP_DEP)], lp).result.type).toBe("ok"); + setMode(MODE.honest); + expect(flashLoan()).toBeOk(Cl.bool(true)); + }); + }); +}