Liquid stacking protocol — deposit STX, receive stSTX, withdraw via NFT receipt
| Severity | Count |
|---|---|
| HIGH | 2 |
| MEDIUM | 2 |
| LOW | 1 |
| INFO | 3 |
StackingDAO v2 is a liquid stacking protocol on Stacks. Users deposit STX and receive stSTX (a fungible token representing their staked position). Withdrawals are a two-phase process: init-withdraw mints an NFT receipt and locks stSTX, then withdraw burns both and returns STX after a PoX cycle boundary. A cancel-withdraw path lets users reclaim stSTX before the unlock height.
Key design: all external contracts (reserve, commission, staking, direct-helpers) are passed as trait parameters and validated against a DAO registry via check-is-protocol. This is a well-known Stacks pattern for upgradeable protocol components.
check-is-protocol contract-callermigrate-ststx) is a one-time administrative operationLocation: set-stack-fee, set-unstack-fee (lines 236–250)
Description: The stack-fee and unstack-fee variables (in basis points) have no upper bound validation. A compromised or malicious DAO governance contract could set fees to 10000 (100%), causing users to lose their entire deposit or withdrawal amount to fees.
(define-public (set-stack-fee (fee uint))
(begin
(try! (contract-call? .dao check-is-protocol contract-caller))
;; No max check — fee could be u10000 (100%)
(var-set stack-fee fee)
(ok true)
)
)Impact: If governance is compromised, all new deposits and withdrawals can be fully drained to fee recipients. Existing stSTX holders' withdrawals would also be affected.
Recommendation: Add a maximum fee constant (e.g., MAX_FEE u500 = 5%) and assert in both setters: (asserts! (<= fee MAX_FEE) (err ERR_MAX_FEE)). Note: v1 had MAX_COMMISSION u2000 (20%) — this guard was removed in v2.
as-contract grants blanket asset authorityLocation: Multiple: deposit (line 88), init-withdraw (line 120), cancel-withdraw (lines 158–160), withdraw (lines 186–194)
Description: The contract uses as-contract (pre-Clarity 4) throughout, which grants the called contract unrestricted access to all assets held by this contract principal. Any trait-parameterized contract called within as-contract blocks (reserve, commission, staking) could potentially move assets beyond what is intended.
Impact: While the DAO registry validation mitigates this (only approved contracts can be passed), if any approved contract has a vulnerability, it could abuse the blanket as-contract authority to drain STX or stSTX held by the core contract.
Recommendation: Migrate to Clarity 4 and use as-contract? with explicit asset allowances: (as-contract? (with-stx ...)). This enforces at the language level which assets can be moved, eliminating the risk even if a registered protocol contract is compromised.
get-withdraw-unlock-burn-heightLocation: get-withdraw-unlock-burn-height (line 49)
Description: The expression (- start-block-next-cycle withdraw-offset) will panic if withdraw-offset exceeds start-block-next-cycle. While this is unlikely in practice (cycle start heights are large numbers), a misconfigured cycle-withdraw-offset in the data contract could cause all deposit, init-withdraw, and cancel-withdraw operations to fail (since they call functions that might reference this read-only).
(if (< burn-block-height (- start-block-next-cycle withdraw-offset))
;; underflow panics if withdraw-offset > start-block-next-cycleImpact: A bad configuration value in .data-core-v1 could DoS the withdrawal timing calculation, though this read-only is only called by init-withdraw.
Recommendation: Use a safe subtraction pattern or validate the offset: (if (> start-block-next-cycle withdraw-offset) (- start-block-next-cycle withdraw-offset) u0)
cancel-withdraw round-trips STX through reserve unnecessarilyLocation: cancel-withdraw (lines 158–160)
Description: To cancel a withdrawal, the contract calls request-stx-for-withdrawal (which transfers STX from reserve to this contract) and immediately sends it back with stx-transfer?. This round-trip exists because request-stx-for-withdrawal is the only way to decrement the stx-for-withdrawals counter. However, this creates a window where reserve funds are temporarily in the core contract, and the two-step operation could fail partway (e.g., if reserve has insufficient unlocked STX).
;; Request STX from reserve (decrements withdrawal counter)
(try! (as-contract (contract-call? reserve request-stx-for-withdrawal stx-amount tx-sender)))
;; Immediately send it back
(try! (as-contract (stx-transfer? stx-amount tx-sender (contract-of reserve))))Impact: If the reserve lacks sufficient unlocked STX to fulfill the request, cancellation fails even though no STX actually needs to move. This could lock users into unwanted withdrawals during periods of high stacking utilization.
Recommendation: Add a dedicated cancel-stx-for-withdrawal function to the reserve trait that decrements the counter without moving STX.
withdraw uses strict > instead of >= for unlock height checkLocation: withdraw (line 183)
Description: The assertion (asserts! (> burn-block-height unlock-burn-height) ...) requires the block height to be strictly greater than the unlock height. If the unlock height is exactly the start of the next cycle, this means users cannot withdraw in the first block of the cycle — they must wait one additional block.
(asserts! (> burn-block-height unlock-burn-height) (err ERR_WITHDRAW_LOCKED))Impact: Minor UX issue — one-block delay on withdrawal availability. Not exploitable.
Recommendation: Consider using >= if the intent is to allow withdrawal once the cycle boundary is reached.
stxstx-amount (typo for ststx-amount)Location: deposit (line 95)
Description: The print statement uses key stxstx-amount instead of ststx-amount. This is inconsistent with other events and could confuse indexers.
(print { action: "deposit", data: { stacker: tx-sender, stx-amount: stx-amount,
stxstx-amount: ststx-amount, ... } })Recommendation: Fix to ststx-amount in a future version for consistency.
Location: deposit (line 76), init-withdraw (line 109)
Description: The stSTX↔STX conversions use integer division which truncates. On deposit: (/ (* stx-user-amount u1000000) stx-ststx). On withdrawal: (/ (* ststx-amount stx-ststx) u1000000). Small rounding losses accumulate in favor of the protocol.
Impact: Dust-level losses per transaction. Standard pattern in DeFi — not a bug, but users should be aware.
Recommendation: Informational only. Consider documenting the rounding behavior.
migrate-ststx has no re-entrancy guard or one-time flagLocation: migrate-ststx (lines 276–286)
Description: The migration function burns stSTX from v1 core and mints to v2 core. It is gated by check-is-protocol but has no flag to prevent multiple calls. If v1 core somehow re-acquires stSTX, migration could be called again.
Impact: Low — would only mint stSTX equal to what v1 core holds, and is admin-gated. But a one-time guard would be cleaner.
Recommendation: Add a migrated boolean flag that prevents re-execution.
check-is-protocol contract-caller.shutdown-deposits provides an emergency brake for new deposits.stack-fee and unstack-fee variables (both default to 0)MAX_COMMISSION constant — fee cap no longer enforced (see H-01)cancel-withdraw function — users can now reverse pending withdrawalsdirect-helpers trait parameter for direct stacking trackingpool parameter to deposit for pool routingmigrate-ststx for v1 → v2 token migrationIndependent audit by cocoa007.btc · Full audit portfolio · Report generated February 25, 2026