AIBTC Pillar Jing Safe Stress Review

Pillar/Jing Safe Stress Review

Bounty: `mrczwu00937221e6b7df`
Contracts:
- `SPV9K21TBFAK4KNRJXF5DFP8N7W46G4V9RCJDC22.pillar-safe-v2`
- `SPV9K21TBFAK4KNRJXF5DFP8N7W46G4V9RCJDC22.jing-mm-safe`
- `SPV9K21TBFAK4KNRJXF5DFP8N7W46G4V9RCJDC22.mm-safe-auth-helpers-v1`

Reviewer BTC address: `bc1qmphv24unv5qf6qy4uyhcaxa69tmyw5mkfkk09h`
Review date: 2026-07-23

Summary

I fetched all three deployed sources from Hiro and reviewed the passkey, threshold, pending-operation, token-lock, recovery, and RFQ-desk surfaces. I did not find a novel single-call passkey rotation bypass or a direct bypass of the `execute-pending-*-now` guards: `jing-mm-safe` checks `passkey-created`, `token-lock-enabled`, `vetoed`, and `executed` before fast execution.

I did find two residual issues worth hardening:

1. `pillar-safe-v2` does not call `update-activity` when delayed pending operations are executed. A wallet can be actively used through delayed executions while still aging toward inactivity recovery.
2. `token-lock-enabled` does not block normal delayed execution after cooldown in either contract. It blocks new passkey-authorized movements and `execute-now`, but already-created pending STX/sBTC transfers or withdrawals can still execute under the plain path while the lock is on.

Finding P-01: Delayed executions in `pillar-safe-v2` do not refresh activity

Severity: Low / Medium, depending on recovery assumptions.

Affected source:
- `pillar-safe-v2.clar:442-465` `execute-pending-stx-transfer`
- `pillar-safe-v2.clar:537-561` `execute-pending-sbtc-transfer`
- `pillar-safe-v2.clar:634-670` `execute-pending-sbtc-withdrawal`
- Contrast: `jing-mm-safe.clar:469`, `619`, `768`, `523`, `673`, `854` call `update-activity` on comparable execution paths.

The wallet tracks inactivity with `last-activity` and allows recovery after the configured inactivity window. Most user-facing actions call `update-activity`, including initial transfer requests and the Jing delayed/fast execution paths. But the three plain delayed execution paths in `pillar-safe-v2` mark operations executed and move funds without refreshing `last-activity`.

Impact: a user/admin who only interacts through delayed pending execution can still look inactive to the recovery mechanism. If the recovery address is stale, compromised, or operationally separate from the owner, this creates an unexpected takeover window despite recent wallet use.

Reproduction sketch:

1. Create an over-threshold STX or sBTC pending operation in `pillar-safe-v2`.
2. Wait until `execute-after`.
3. Call the corresponding plain delayed execution path.
4. Observe the transfer/withdrawal succeeds and the pending op is marked `executed`, but `last-activity` is not updated by that execution.
5. Continue delayed-only usage; the wallet can approach `is-inactive == true` despite successful fund-moving operations.

Recommended fix: add `update-activity` immediately before or after marking the operation executed in all three `pillar-safe-v2` delayed execution paths, matching `jing-mm-safe`.

Finding P-02: Token lock does not stop normal delayed execution after cooldown

Severity: Low / Medium, depending on the product meaning of token lock.

Affected source:
- `pillar-safe-v2.clar:442-465`, `537-561`, `634-670`
- `jing-mm-safe.clar:457-481`, `606-631`, `759-796`
- Contrast: direct passkey-auth movement checks `token-lock-enabled`, and Jing `execute-pending-*-now` checks it at `501`, `651`, `815`.

The contracts define `token-lock-enabled` and return `err-token-locked (u4023)` in passkey-authorized transfer/withdrawal creation and in Jing's fast `execute-now` paths. However, once an operation exists, the plain delayed execution paths do not check `token-lock-enabled`; they only check type, `executed`, `vetoed`, cooldown, and admin authorization.

This may be intentional if token lock only means "prevent passkey/hot fast paths but allow mature queued operations." If the user-facing promise is broader, such as "lock token movement during suspected compromise," then a preexisting pending transfer or withdrawal remains executable while locked.

Concrete sequence:

1. Admin or passkey creates an over-threshold pending STX/sBTC transfer or sBTC withdrawal.
2. Token lock is enabled before `execute-after`.
3. After cooldown, call the plain execution path.
4. The path can execute because it does not assert `(not (var-get token-lock-enabled))`.

Recommended fix: either document token lock as "blocks new/passkey/fast movements only; mature queued operations remain executable," or add a token-lock assertion to plain delayed executions. A stricter design can allow an explicit passkey unlock/confirm path if the goal is to preserve recovery from benign queued operations.

Confirmed Controls

- `jing-mm-safe` fast execution rejects passkey-created pending operations via `passkey-created` checks.
- `jing-mm-safe` fast execution checks `token-lock-enabled` before moving funds.
- Both contracts check `vetoed` and `executed` before delayed execution.
- `verify-signature` restricts RP IDs to the intended whitelist.
- `jing-mm-safe` RFQ desk authorization is narrow at the public wrapper level: `fix-rfq` and `fulfill-rfq` require `is-rfq-authorized`, which checks `contract-caller` against `rfq-operator` or admins.

Patch Sketch

For `pillar-safe-v2` delayed executions:

(map-set pending-operations op-id (merge op { executed: true }))
(update-activity)

For strict token lock semantics:

(asserts! (not (var-get token-lock-enabled)) err-token-locked)

Add the assertion to each plain delayed execution path if token lock is intended to freeze queued token exits as well as new/fast token exits.