Skip to content

Commit 4533816

Browse files
UltraDAGcomClaude Opus 4.7 (1M context)
andcommitted
fix(state): rebuild pocket_to_parent index on load (INTERNAL-2026-04-22-persist)
pocket_to_parent is a derived in-memory reverse index (pocket_addr → parent_addr) maintained incrementally on CreatePocket / RemovePocket. It is not persisted to redb — the authoritative data is the SmartAccountConfig.pockets list (of labels) on each parent. rebuild_pocket_map() was defined on StateEngine but never called. After any node restart, pocket_to_parent was empty, which caused: 1. Every pocket unspendable: verify_smart_transfer's parent-fallback couldn't resolve pocket → parent. 2. GHSA-9chc-gjfr-6hrq silent regression: check_spending_policy's unwrap_or(*from) fell through to the pocket's (empty) config, bypassing every limit the user had set on the parent. Fix: call engine.rebuild_pocket_map() in load_from_redb immediately after restore_smart_accounts(). Three regression tests in crates/ultradag-coin/tests/pocket_persistence.rs exercise save → reload round-trip: index is rebuilt, pocket remains spendable, and the parent's daily_limit continues to apply to pocket-originated transfers after reload (5/5 assertions). Found by the second-pass security audit launched after the GHSA-9chc and INTERNAL-2026-04-22-pocket-keyreg fixes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 461eb1a commit 4533816

4 files changed

Lines changed: 228 additions & 1 deletion

File tree

crates/ultradag-coin/src/state/db.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -558,9 +558,16 @@ pub fn load_from_redb(path: &Path) -> Result<StateEngine, PersistenceError> {
558558
engine.restore_bridge_state(bridge_attestations, bridge_sigs, bridge_nonce);
559559
engine.set_bridge_contract_address(bridge_contract_address);
560560

561-
// Restore SmartAccounts
561+
// Restore SmartAccounts. The pocket_to_parent reverse-index is derived
562+
// state — it lives in memory only, not in redb — so it must be rebuilt
563+
// from each config's pockets list immediately after restore. Without
564+
// this, every pocket on the node becomes unspendable after a restart
565+
// (verify_smart_transfer's parent-fallback finds no entry) and spending
566+
// policies silently stop being enforced (check_spending_policy's
567+
// pocket→parent resolution falls through to the pocket's empty config).
562568
if !smart_accounts_vec.is_empty() {
563569
engine.restore_smart_accounts(smart_accounts_vec);
570+
engine.rebuild_pocket_map();
564571
}
565572

566573
// Restore Name Registry
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
//! Regression tests for the pocket_to_parent rebuild-on-load contract.
2+
//!
3+
//! `pocket_to_parent` is a derived in-memory reverse index (pocket_addr →
4+
//! parent_addr). It is NOT persisted to redb — only the owning parent's
5+
//! `SmartAccountConfig.pockets` list (of labels) is. So after every node
6+
//! restart, the map must be rebuilt from those labels, or:
7+
//!
8+
//! 1. `verify_smart_transfer` for pocket-originated transfers fails at the
9+
//! parent-fallback step → every pocket becomes unspendable.
10+
//! 2. `check_spending_policy` cannot resolve pocket→parent → falls through
11+
//! to the pocket's (empty) config → GHSA-9chc-gjfr-6hrq re-emerges.
12+
//!
13+
//! These tests save a StateEngine with a funded pocket + active policy to a
14+
//! redb file, reload into a fresh engine, and assert both properties hold.
15+
16+
use tempfile::NamedTempFile;
17+
use ultradag_coin::address::{Address, SecretKey, Signature};
18+
use ultradag_coin::state::{db, StateEngine};
19+
use ultradag_coin::tx::name_registry::derive_pocket_address;
20+
use ultradag_coin::tx::smart_account::*;
21+
22+
fn seed_engine_with_pocket() -> (StateEngine, SecretKey, Address) {
23+
let alice = SecretKey::from_bytes([0xA1; 32]);
24+
let mut engine = StateEngine::new_with_genesis();
25+
engine.faucet_credit(&alice.address(), 20_000_000_000).unwrap();
26+
engine.ensure_smart_account(&alice.address());
27+
28+
// Seed parent's Ed25519 key so the parent config has a real authorized
29+
// key that can sign pocket transfers after reload.
30+
let pubkey = alice.verifying_key().to_bytes();
31+
let key_id = AuthorizedKey::compute_key_id(KeyType::Ed25519, &pubkey);
32+
let cfg = engine.smart_account_mut_for_test(&alice.address()).unwrap();
33+
cfg.authorized_keys.push(AuthorizedKey {
34+
key_id, key_type: KeyType::Ed25519, pubkey: pubkey.to_vec(),
35+
label: "owner".to_string(), daily_limit: None, daily_spent: (0, 0),
36+
});
37+
38+
// Install a strict parent policy that will be enforced on pocket spends.
39+
let mut policy_tx = SetPolicyTx {
40+
from: alice.address(),
41+
instant_limit: 100_000_000,
42+
vault_threshold: 0,
43+
vault_delay_rounds: 0,
44+
whitelisted_recipients: vec![],
45+
daily_limit: Some(100_000_000), // 1 UDAG/day on the parent
46+
fee: 0, nonce: 0, // fee 0 keeps the supply invariant simple in-test
47+
pub_key: pubkey,
48+
signature: Signature([0u8; 64]),
49+
};
50+
policy_tx.signature = alice.sign(&policy_tx.signable_bytes());
51+
engine.apply_set_policy_tx(&policy_tx, 100).unwrap();
52+
engine.process_pending_policy_changes(100 + POLICY_CHANGE_DELAY_ROUNDS);
53+
54+
// Create + fund the pocket.
55+
let op = SmartOpTx {
56+
from: alice.address(),
57+
operation: SmartOpType::CreatePocket { label: "savings".to_string() },
58+
fee: 0, nonce: 1, signing_key_id: [0u8; 8],
59+
signature: vec![], webauthn: None, p256_pubkey: None,
60+
};
61+
engine.apply_smart_op_tx(&op, 100).unwrap();
62+
let pocket = derive_pocket_address(&alice.address(), "savings");
63+
engine.faucet_credit(&pocket, 5_000_000_000).unwrap();
64+
65+
(engine, alice, pocket)
66+
}
67+
68+
fn save_and_reload(engine: &StateEngine) -> StateEngine {
69+
let tmp = NamedTempFile::new().unwrap();
70+
let path = tmp.path().to_path_buf();
71+
drop(tmp); // redb wants to own the file
72+
db::save_to_redb(engine, &path).unwrap();
73+
db::load_from_redb(&path).unwrap()
74+
}
75+
76+
#[test]
77+
fn pocket_to_parent_map_rebuilt_on_reload() {
78+
let (engine, alice, pocket) = seed_engine_with_pocket();
79+
assert_eq!(engine.pocket_parent(&pocket), Some(alice.address()));
80+
81+
let reloaded = save_and_reload(&engine);
82+
assert_eq!(
83+
reloaded.pocket_parent(&pocket),
84+
Some(alice.address()),
85+
"pocket_to_parent must be rebuilt from SmartAccountConfig.pockets after load",
86+
);
87+
}
88+
89+
#[test]
90+
fn parent_policy_still_enforced_on_pocket_after_reload() {
91+
// Before the fix, pocket_to_parent was empty after load, so
92+
// check_spending_policy fell through to Ok(None) and the parent's
93+
// daily_limit was silently skipped for pocket-originated transfers.
94+
let (engine, alice, pocket) = seed_engine_with_pocket();
95+
let bob = Address([0xB0; 20]);
96+
97+
let mut reloaded = save_and_reload(&engine);
98+
99+
// First 1 UDAG hits the parent's cap exactly.
100+
let pubkey = alice.verifying_key().to_bytes();
101+
let key_id = AuthorizedKey::compute_key_id(KeyType::Ed25519, &pubkey);
102+
let make = |amount: u64, nonce: u64| {
103+
let mut tx = SmartTransferTx {
104+
from: pocket, to: bob, amount, fee: 0, nonce,
105+
signing_key_id: key_id, signature: vec![],
106+
memo: None, webauthn: None,
107+
};
108+
tx.signature = alice.sign(&tx.signable_bytes()).0.to_vec();
109+
tx
110+
};
111+
112+
reloaded.apply_smart_transfer_tx(&make(100_000_000, 0)).unwrap();
113+
114+
// Second tx must hit the PARENT's daily_spent counter and be rejected.
115+
let err = reloaded.apply_smart_transfer_tx(&make(1_000_000_000, 1)).unwrap_err();
116+
assert!(
117+
err.to_string().contains("daily spending limit exceeded"),
118+
"parent policy must carry across reload; got: {err}",
119+
);
120+
}
121+
122+
#[test]
123+
fn pocket_is_spendable_after_reload() {
124+
// Before the fix, verify_smart_transfer's pocket→parent fallback failed
125+
// because pocket_to_parent was empty, making every pocket unspendable.
126+
let (engine, alice, pocket) = seed_engine_with_pocket();
127+
let bob = Address([0xB1; 20]);
128+
let reloaded = save_and_reload(&engine);
129+
130+
let pubkey = alice.verifying_key().to_bytes();
131+
let key_id = AuthorizedKey::compute_key_id(KeyType::Ed25519, &pubkey);
132+
let mut tx = SmartTransferTx {
133+
from: pocket, to: bob, amount: 50_000_000, fee: 0, nonce: 0,
134+
signing_key_id: key_id, signature: vec![],
135+
memo: None, webauthn: None,
136+
};
137+
tx.signature = alice.sign(&tx.signable_bytes()).0.to_vec();
138+
139+
assert!(
140+
reloaded.verify_smart_transfer(&tx),
141+
"pocket-originated transfer must verify against the parent's keys after reload",
142+
);
143+
}
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
# INTERNAL-2026-04-22 — Pocket persistence gap re-exposes pocket-policy bypass after restart
2+
3+
**Severity:** Critical
4+
**Component:** `ultradag-coin` — StateEngine persistence (redb load path)
5+
**Disclosure:** Internal review (second-pass audit; no bounty payout)
6+
**Status:** Fixed
7+
8+
## Summary
9+
10+
`pocket_to_parent: HashMap<Address, Address>` is the authoritative reverse
11+
index that policy enforcement and signature authorization depend on for
12+
every pocket-originated transfer. It is **derived** state, maintained
13+
incrementally on `CreatePocket` / `RemovePocket`, and deliberately *not*
14+
persisted to redb — the authoritative data is the `pockets: Vec<String>`
15+
list on each parent's `SmartAccountConfig`.
16+
17+
The `rebuild_pocket_map()` helper that reconstructs this index from those
18+
lists was defined on `StateEngine` (line 376) but was **never called
19+
anywhere in the codebase**. A source-level comment at the `from_parts`
20+
constructor even stated the invariant ("Rebuilt via rebuild_pocket_map()
21+
after loading") — the call simply wasn't wired. After any node restart,
22+
`pocket_to_parent` was empty.
23+
24+
## Impact
25+
26+
On every node restart:
27+
28+
1. **Every pocket becomes unspendable.** `verify_smart_transfer` checks
29+
the pocket's own config first (empty) then falls back to
30+
`pocket_to_parent.get(&tx.from)` — which returns `None` with no map
31+
loaded. The signature does not verify against either surface and the
32+
transfer is rejected. Real user funds are locked on the live pocket
33+
address.
34+
35+
2. **The GHSA-9chc-gjfr-6hrq fix silently regresses.** Once
36+
`pocket_to_parent` is empty, `check_spending_policy`'s `unwrap_or(*from)`
37+
falls through to the pocket's own `SmartAccountConfig`, which has no
38+
policy. Every account-level limit (daily, vault threshold, whitelist)
39+
that a user had configured on their parent account stops being enforced
40+
for any of their pockets.
41+
42+
Combined with INTERNAL-2026-04-22-pocket-keyreg (the key-injection fix
43+
landed earlier today in commit `826e2a28`), these three together form a
44+
complete defense for pockets. This bug would have undermined the other
45+
two on the first restart.
46+
47+
## Root cause
48+
49+
A function that is load-bearing for correctness was defined but never
50+
called. Runtime mutations to the map stayed correct because `CreatePocket`
51+
and `RemovePocket` update it incrementally. The gap only opened at startup
52+
— exactly when no test suite happened to exercise a save/load round-trip
53+
with an active pocket.
54+
55+
## Fix
56+
57+
One-line: call `engine.rebuild_pocket_map()` in
58+
`crates/ultradag-coin/src/state/db.rs::load_from_redb` immediately after
59+
`engine.restore_smart_accounts(smart_accounts_vec)`.
60+
61+
Three regression tests in
62+
`crates/ultradag-coin/tests/pocket_persistence.rs`:
63+
64+
- `pocket_to_parent_map_rebuilt_on_reload` — direct assertion that the
65+
index is non-empty after save/load.
66+
- `pocket_is_spendable_after_reload``verify_smart_transfer` succeeds
67+
for a pocket-originated transfer after reload.
68+
- `parent_policy_still_enforced_on_pocket_after_reload` — the parent's
69+
daily_limit continues to apply to pocket-originated transfers after
70+
reload; a follow-up transfer past the cap is rejected.
71+
72+
## Credits
73+
74+
Found by the second-pass state-engine audit launched after the first two
75+
pocket fixes landed (commit `fb6ef59d` for GHSA-9chc-gjfr-6hrq, commit
76+
`826e2a28` for INTERNAL-2026-04-22-pocket-keyreg).

docs/security/advisories/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ This directory contains published security advisories for vulnerabilities that h
66

77
- [GHSA-9chc-gjfr-6hrq](GHSA-9chc-gjfr-6hrq.md) — Critical — Spending-policy bypass via pockets. SmartAccount policies (daily limit, vault threshold, whitelist, per-key limit) were not enforced on transfers originating from pocket sub-addresses. Reported by Sumitshah00, fixed 2026-04-21.
88
- [INTERNAL-2026-04-22-pocket-keyreg](INTERNAL-2026-04-22-pocket-keyreg.md) — Critical — Pocket key-injection enables pocket drain. `auto_register_ed25519_key` did not require the pubkey to derive to the target address, so any attacker could plant their key on a victim's pocket and then spend from it. Found by internal review, fixed 2026-04-22.
9+
- [INTERNAL-2026-04-22-pocket-persist](INTERNAL-2026-04-22-pocket-persist.md) — Critical — Pocket persistence gap. `pocket_to_parent` reverse-index was never rebuilt on node restart; after any restart every pocket became unspendable and the GHSA-9chc policy-bypass silently regressed. Found by second-pass internal review, fixed 2026-04-22.
910

1011
## Disclosure Timeline
1112

0 commit comments

Comments
 (0)