Skip to content

fix(F-9): add reentrancy locks to the v3 successor pools (Option B) - #88

Open
unixwhisperer wants to merge 1 commit into
mainfrom
security-lead/f9-v3-successor-reentrancy-lock
Open

unixwhisperer wants to merge 1 commit into
mainfrom
security-lead/f9-v3-successor-reentrancy-lock

Conversation

@unixwhisperer

Copy link
Copy Markdown
Collaborator

Summary

Implements Flashstack-ajv.4.9 per your Option B decision (email, 2026-10-05).

  • Lock: flashstack-stx-pool-v3 and flashstack-sbtc-pool-v3 get pool-v3's pv3-F1 guard as one reentrancy-locked bool per pool, shared by deposit, withdraw and flash-loan. Each entry point checks and sets it first and clears it just before its (ok ...). A blocked call returns the new ERR-REENTRANT (u411 STX / u712 sBTC), which is unique within each contract.
  • Deposit order: deposit now records shares before the token transfer, matching pool-v3. This is behaviour-neutral: new-shares and current-shares are bound in the let before either order, so the values written are identical. A failed transfer makes the public function return err, which rolls back every write in the call, the lock included. The lock blocks reentry either way.
  • Unchanged: the flash-loan repayment check (reserve-after >= reserve-before + fee), fee logic, pause gating, admin and receiver approval are all untouched. The only removed lines in the diff are deposit's two moved statements.
  • Test copies: the contracts/test/ copies were regenerated from the canonical sources and differ only in principal rewrites.
  • Test receivers: two new test-only receivers, test-{stx,sbtc}-pool-v3-receiver-reentrant, with five callback modes: honest repay, reenter deposit, reenter withdraw, under-repay, nested flash-loan.
  • Docs: the F-9 entry in FINDINGS_REGISTER.md is updated.

Verification

All of this ran in a clean git worktree at origin/main e0367e006c45 (fresh npm ci, clarinet 3.23.2 as in CI). The commit's tree hash (1c9db567) is identical to the verified tree.

Check Baseline origin/main This PR
clarinet check exit 0, 213 contracts exit 0, 215 contracts (+2 receivers), 0 errors; all 6 touched/new files also check clean individually
Full suite (CLARINET_BIN=… npx vitest run) 27 files, 265 passed + 1 expected fail 28 files, 279 passed + 1 expected fail. The +14 is exactly the new file (7 tests × 2 pools)
tests/v3-pools-reentrancy-lock.test.ts — 14/14
Same file against the pre-fix pool sources — 8 fail / 6 pass. All lock-dependent tests fail (deposit-reentry, zero-mutation, withdraw-reentry, lock release; 4 per pool). Happy path, under-repay and the nested-loan case pass, as they should
mainnet-plan-guard (ran, not skipped) 2 passed + 1 expected fail 2 passed + 1 expected fail
Test-copy equivalence — Regenerated output is byte-identical to the committed copies. A separate normalizer finds canonical == copy, and the substituted lines are the same set as on main (1 STX, 13 sBTC)

Per pool, the suite covers:

  • happy path (flash-loan, deposit, withdraw in sequence, with stats)
  • deposit-reentry rejected
  • the same attempt leaves pool balance, stats, total-shares and both positions unchanged
  • withdraw-reentry rejected with the receiver's seeded shares untouched
  • lock released after a blocked reentry
  • lock released after an under-repay revert (then withdraw, deposit and loan all work)

The negative cases assert the exact ERR-REENTRANT code, which nothing else on that path produces. The withdraw case first asserts that the receiver really holds shares, so it can't pass on a zero-amount error.

One finding from writing the tests: a nested flash-loan on the same pool never reaches the lock. The Clarity VM aborts it as RuntimeCheck(CircularReference), with or without this fix. The test pins that behaviour rather than crediting it to the lock.

Security notes

🤖 Generated with Claude Code

flashstack-stx-pool-v3 and flashstack-sbtc-pool-v3 shipped without the
reentrancy guard (PR #84), so a flash-loan receiver could still "repay" by
calling deposit mid-callback and mint shares at the loan-depressed price.

Port pool-v3's pv3-F1 guard as one `reentrancy-locked` bool per pool,
shared by deposit, withdraw and flash-loan (Option B, Matt 2026-10-05),
returning ERR-REENTRANT (u411 STX / u712 sBTC). Deposit now records shares
before the transfer, matching pool-v3; a failed transfer still reverts the
whole call. The flash-loan repayment check is unchanged.

contracts/test/ copies regenerated from the canonical sources (principal
rewrites only). New test receivers drive honest, deposit-reentry,
withdraw-reentry, under-repay and nested-loan callbacks against each pool;
registered in Clarinet.toml and mainnet-plan-guard's known set.

Both pools remain undeployed: this is a repository fix, not a remediation
of any deployed contract.
@vercel

vercel Bot commented Oct 5, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
web Ready Ready Preview Oct 5, 2026 7:07am UTC

Request Review

This branch was successfully deployed

1 active deployment
Preview — 9eef5911 Deployed Oct 5, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant