Arkadiko Stacker v3-1

Location: return-stx function (lines 133-139)

Description: The return-stx function has no authorization check. Any user can call it to transfer any amount of the contract's unlocked STX back to the reserve. While STX goes to the reserve (not the attacker), this allows anyone to disrupt stacking operations by draining the contract's available balance before stack-increase or initiate-stacking can be called.

;; No access control — anyone can call this
(define-public (return-stx (ustx-amount uint))
  (begin
    (as-contract
      (stx-transfer? ustx-amount tx-sender
        (unwrap-panic (contract-call? .arkadiko-dao get-qualified-name-by-name "stx-reserve")))
    )
  )
)

Impact: An attacker can front-run stack-increase calls by calling return-stx with the contract's full balance, sending STX back to the reserve and causing the stacking increase to fail. This creates a griefing vector that prevents the protocol from increasing its stacking position. During the window between request-stx-to-stack (which transfers STX to this contract) and the actual PoX lock, an attacker could repeatedly drain funds back to the reserve.

Recommendation: Add DAO owner or guardian authorization:

(define-public (return-stx (ustx-amount uint))
  (begin
    (asserts! (is-eq tx-sender (contract-call? .arkadiko-dao get-dao-owner)) (err ERR-NOT-AUTHORIZED))
    (as-contract
      (stx-transfer? ustx-amount tx-sender
        (unwrap-panic (contract-call? .arkadiko-dao get-qualified-name-by-name "stx-reserve")))
    )
  )
)

Location: initiate-stacking (lines 62-68, 78-80), stack-increase (lines 95-98)

Description: When PoX calls fail, the error branches use print to log the error but the match expression's error branch returns the print result, not a proper (err ...). The print function returns its argument, so these branches return an (err uint) value — but only from the inner match. The outer match in initiate-stacking wraps this in another print, which returns the error value through the function. However, the can-stack-stx failure path returns the print of an error, which Clarity treats as a successful (ok ...) response from the outer let.

;; In initiate-stacking — failure of can-stack-stx:
(match (as-contract (contract-call? ... can-stack-stx ...))
  success (begin ...)
  failure (print (err (to-uint failure)))  ;; returns (err uint) — but the function signature expects (response uint uint)
                                            ;; This actually propagates correctly as (err)
)

Impact: The initiate-stacking function returns (ok u0) when get-tokens-to-stack returns an error (line 51: (unwrap! ... (ok u0))), masking a real failure as success. The DAO owner may believe stacking was initiated when no tokens were available. State variables remain unchanged, but callers receive a success response.

Recommendation: Return proper error codes instead of (ok u0) on failure. Replace (unwrap! ... (ok u0)) with (unwrap! ... (err ERR-NO-TOKENS)). Ensure all match error branches explicitly return (err ...).

Location: All PoX calls (lines 58, 60, 87, 107)

Description: The contract hardcodes calls to SP000000000000000000002Q6VF78.pox-3. Stacks has already upgraded to pox-4, and this contract can no longer initiate new stacking or extend cycles. Any call to stack-stx, stack-increase, or stack-extend on pox-3 will fail after the PoX upgrade cutoff.

(as-contract (contract-call? 'SP000000000000000000002Q6VF78.pox-3 stack-stx ...))

Impact: The contract is functionally deprecated. Once the current stacking cycle on pox-3 expires, STX cannot be re-stacked. This is a design limitation of pre-trait PoX integration — a new stacker contract (v4+) would be needed for each PoX upgrade.

Recommendation: Deploy a new stacker version targeting pox-4. Consider using a DAO-upgradeable proxy pattern or trait-based PoX interface for future versions.

Location: All as-contract usages (lines 58, 60, 87, 107, 135)

Description: The contract uses the legacy as-contract builtin, which grants unrestricted authority to move any asset held by the contract within the enclosed expression. In Clarity 4, as-contract? with explicit with-stx / with-stacking allowances would limit operations to only the intended asset types.

Impact: If any of the called contracts (pox-3, arkadiko-dao, arkadiko-stx-reserve-v1-1) were compromised or had unexpected callbacks, as-contract would allow them to move any token the stacker contract holds, not just STX. In practice the risk is limited since the called contracts are well-known boot/protocol contracts, but it violates least-privilege.

Recommendation: In the next version, use Clarity 4's as-contract?:

(as-contract? (with-stx (with-stacking
  (contract-call? 'SP000000000000000000002Q6VF78.pox-4 stack-stx ...))))

Location: return-stx (line 137)

Description: The return-stx function uses unwrap-panic to resolve the DAO's qualified name lookup. If get-qualified-name-by-name returns none (e.g., the "stx-reserve" key is removed from the DAO registry), the entire transaction aborts with a runtime panic rather than returning a graceful error.

(unwrap-panic (contract-call? .arkadiko-dao get-qualified-name-by-name "stx-reserve"))

Impact: Low — the DAO registry is unlikely to remove "stx-reserve", but the pattern is fragile. If the registry is misconfigured, STX cannot be returned from this contract.

Recommendation: Use unwrap! with a proper error code instead of unwrap-panic.

Location: Line 17

Description: stacker-name is declared as a data-var (mutable), but no function in the contract updates it. It is always "stacker". This could be a define-constant instead.

Impact: No security impact. Minor code quality issue — using define-constant would save gas on reads and make immutability explicit.

Recommendation: Change to (define-constant STACKER-NAME "stacker").

Location: initiate-stacking, stack-increase, stack-extend

Description: The state variables stacking-unlock-burn-height and stacking-stx-stacked are only updated on success. If a PoX call fails after request-stx-to-stack succeeds in initiate-stacking, the STX has been transferred to this contract but the state variables still show 0. However, since the PoX failure causes the error to propagate (Clarity rolls back on error), the request-stx-to-stack transfer is also rolled back. The state remains consistent.

Impact: None — Clarity's atomic transaction semantics ensure consistency. Noted for documentation purposes: the try! on request-stx-to-stack ensures rollback on subsequent failure.

;; Exploit test for C-01: return-stx has no access control
;; Any user can call return-stx to drain unlocked STX from the stacker contract

(define-public (test-exploit-c01-return-stx-no-auth)
  (let (
    ;; Assume the stacker contract has some unlocked STX
    ;; (e.g., after stacking cycle ends or between request-stx-to-stack and stack-stx)
    (balance-before (stx-get-balance (as-contract tx-sender)))
  )
    ;; Attacker (any tx-sender) calls return-stx
    ;; This should fail with ERR-NOT-AUTHORIZED but currently succeeds
    (try! (contract-call? .arkadiko-stacker-v3-1 return-stx balance-before))

    ;; Verify: contract balance is now 0 — STX sent to reserve without authorization
    (asserts! (is-eq (stx-get-balance (as-contract tx-sender)) u0) (err u999))
    (ok true)
  )
)