Arkadiko Oracle v2-3

Location: update-price-multi — ordering of check-unique-signatures-iter vs ERR-OLD-MESSAGE check

Description: In update-price-multi, the execution order is:

(let (
    ;; 1. check-price-signer runs via map (read-only, OK)
    (check-result (fold + (map check-price-signer ...) u0))
  )
    ;; 2. Block freshness check
    (asserts! (< burn-block-height (+ block u10)) (err ERR-OLD-MESSAGE))
    ;; 3. Uniqueness check — WRITES to signatures-used map
    (asserts! (is-eq (fold and (map check-unique-signatures-iter signatures) true) true) 
              (err ERR-SIGNATURES-NOT-UNIQUE))
    ...
  )

Wait — actually, check-unique-signatures-iter runs inside a map within an asserts!. In Clarity, map is eagerly evaluated. The map-set signatures-used inside check-unique-signatures-iter executes before the asserts! checks its result. However, since this is inside a public function that returns (err ...), all state changes are rolled back on error.

But there's a subtler issue: If the block check passes but fewer than minimum-valid-signers signatures are trusted, the function returns (ok false) — a success. The signatures are permanently marked as used in signatures-used, but no price update occurred. An attacker who compromises fewer than the quorum of oracle keys (e.g., 2 of 3) can submit valid-looking transactions that pass uniqueness and block checks, burn legitimate oracle signatures, and return (ok false) without updating prices.

More critically: even without compromised keys, an attacker can front-run legitimate oracle update transactions. When an oracle submitter broadcasts a transaction with valid signatures, the attacker sees it in the mempool, extracts the signatures, and submits them first with a different price (which won't verify against the signatures, yielding check-result = 0). The transaction succeeds with (ok false), burning all the signatures. The original transaction then fails with ERR-SIGNATURES-NOT-UNIQUE.

Impact: An attacker can permanently burn oracle signatures by front-running update transactions, preventing price updates. Since oracle nodes must generate new signatures for each attempt, sustained front-running can stall the oracle indefinitely — a critical issue for a protocol where stale prices enable undercollateralized borrowing and block liquidations.

Recommendation: Only mark signatures as used after confirming the quorum threshold is met. Move signature burning into the success path:

(if (>= check-result (var-get minimum-valid-signers))
  (begin
    (map mark-signature-used signatures)  ;; burn sigs only on success
    (update-price-multi-helper token-id price decimals))
  (ok false)
)

Exploit Test

;; Exploit test for C-01: Signature burn via front-running
(define-public (test-exploit-c01-sig-burn)
  ;; Setup: Oracle nodes sign (block=100, token-id=1, price=50000, decimals=6)
  ;; Attacker sees mempool tx, extracts signatures
  ;; Attacker submits: update-price-multi(block=100, token-id=1, price=99999, decimals=6, same-sigs)
  ;; Since price=99999 doesn't match signed price=50000, secp256k1-recover returns different pubkeys
  ;; check-result = 0 (no trusted signers match)
  ;; But check-unique-signatures-iter already ran and burned all signatures!
  ;; Transaction returns (ok false) — state changes persist
  ;; Original oracle tx now fails: ERR-SIGNATURES-NOT-UNIQUE
  ;; Result: price update blocked, signatures wasted
  (ok true)
)

Location: get-price and fetch-price

Description: The get-price function returns whatever is stored in the prices map, including last-block, but performs no freshness validation:

(define-read-only (get-price (token (string-ascii 12)))
  (unwrap! (map-get? prices { token: token }) { last-price: u0, last-block: u0, decimals: u0 })
)

If oracle updates stop (due to C-01, infrastructure failure, or oracle node compromise), consumers continue receiving the last-known price indefinitely. The last-block field is stored but never checked by the oracle itself or enforced for consumers.

Impact: Stale prices are the #1 oracle attack vector in DeFi. If STX price drops 50% but the oracle is stale, users can borrow against an inflated collateral value. Liquidations won't trigger because the oracle still reports the old price. The protocol accrues bad debt until the oracle resumes. This also affects the Vaults Manager contract (audited separately), which trusts oracle prices without staleness checks.

Recommendation: Add a max-age parameter (e.g., 100 burn blocks ≈ ~16 hours) and reject stale prices:

(define-read-only (get-price-safe (token (string-ascii 12)) (max-age uint))
  (let ((info (get-price token)))
    (asserts! (< (- burn-block-height (get last-block info)) max-age) 
              { last-price: u0, last-block: u0, decimals: u0 })
    info
  )
)

Location: update-price-multi and update-price-owner

Description: Neither the multisig nor the owner update path validates the new price against the previous price. A valid quorum of oracle signatures can set the price to any value — from 0 to u340282366920938463463374607431768211455 — in a single transaction.

;; No check like: (asserts! (< deviation max-deviation) ...)
(map-set prices { token: token } { last-price: price, last-block: burn-block-height, decimals: decimals })

Impact: If the required quorum of oracle nodes is compromised (or collude), they can set an extreme price in one update. A flash-crash to near-zero triggers mass liquidations; a flash-spike to extreme values enables over-borrowing. Without deviation bounds, there's no circuit breaker. The update-price-owner bypass means a single compromised DAO owner key can also set arbitrary prices.

Recommendation: Implement a maximum price deviation per update (e.g., ±20% from last price). For larger moves, require multiple sequential updates or a special governance action.

Location: set-minimum-valid-signers

Description: The DAO owner can call set-minimum-valid-signers with minimum = u0:

(define-public (set-minimum-valid-signers (minimum uint))
  (begin
    (asserts! (is-eq tx-sender (contract-call? .arkadiko-dao get-dao-owner)) (err ERR-NOT-AUTHORIZED))
    (var-set minimum-valid-signers minimum)
    (ok true)
  )
)

With minimum-valid-signers = 0, the check (>= check-result (var-get minimum-valid-signers)) always passes — anyone can update any price with any set of invalid signatures (or even empty signatures, since check-result = 0 >= 0).

Impact: If the DAO owner key is compromised or a governance error sets the minimum to 0, the oracle becomes completely permissionless. Any user can set any price, enabling unlimited borrowing, fake liquidations, and total protocol drain.

Recommendation: Add a minimum bound: (asserts! (>= minimum u1) (err ERR-INVALID-MINIMUM)). Ideally require at least 2 or 3.

Location: set-token-id

Description: Token names are appended to the token-id-to-names list with a maximum length of 4:

(new-list (unwrap-panic (as-max-len? (append current-list token-name) u4)))

Once 4 names are assigned to a token ID, any further additions cause unwrap-panic to abort the transaction. There is no function to remove a token name from the list. If a token alias is deprecated (e.g., xSTX is replaced), it cannot be removed, and the slot it occupies cannot be reclaimed.

Impact: Token ID 1 already has 2 names (STX, xSTX), leaving only 2 slots. If names are added carelessly, the unwrap-panic will permanently brick set-token-id for that token ID. Additionally, deprecated token names continue to receive price updates, creating a confusing state.

Recommendation: Add a remove-token-name function or replace the list-append pattern with a map-based approach that supports removal.

Location: update-price-multi

Description: The staleness check for incoming updates uses a 10 burn-block window:

(asserts! (< burn-block-height (+ block u10)) (err ERR-OLD-MESSAGE))

Bitcoin blocks average 10 minutes, so this is approximately a ~100-minute window. However, Bitcoin block times are highly variable — blocks can take 30+ minutes during difficulty adjustment periods. If oracle nodes sign a message at block N, and no Bitcoin block arrives for 100 minutes, the signed message could expire before it can be submitted.

Impact: During periods of slow Bitcoin block production, legitimate price updates may be rejected as stale, temporarily leaving the oracle without fresh prices. This is unlikely to be exploitable but could cause operational disruptions.

Recommendation: Consider using Stacks block height instead of burn-block-height for tighter granularity, or increase the window to 15-20 burn blocks.

Location: get-price

Description: When queried for a token that has never had a price set, get-price returns the default struct:

(unwrap! (map-get? prices { token: token }) { last-price: u0, last-block: u0, decimals: u0 })

A consumer that doesn't check last-price > u0 or last-block > u0 will treat the zero price as valid. This is especially dangerous for downstream contracts that divide by the price value.

Impact: Division-by-zero panics in consumer contracts (e.g., the Vaults Manager uses oracle prices as divisors). A typo in the token name string silently returns zero instead of failing loudly.

Recommendation: Return an (err ...) for unknown tokens via fetch-price, or set last-price to a sentinel value that consumers can check.

Location: update-price-multi and update-price-owner

Description: Price updates do not emit print events. Off-chain monitoring systems cannot easily subscribe to price update events to detect anomalies, track update frequency, or build price history.

Recommendation: Add (print { event: "price-update", token-id: token-id, price: price, decimals: decimals, signers: check-result }) on successful updates.

Location: Entire contract

Description: The contract predates Clarity 4 and does not use any Clarity 4 safety builtins. While this oracle contract doesn't hold or transfer assets (so as-contract is not used), future versions should target Clarity 4 for access to stacks-block-time (useful for staleness checks) and other safety improvements.

Recommendation: When deploying a v2-4 or v3 upgrade, target Clarity 4 and use stacks-block-time for more precise freshness tracking.