Skip to content

Commit 826e2a2

Browse files
UltraDAGcomClaude Opus 4.7 (1M context)
andcommitted
fix(smart-account): block pocket key-injection (INTERNAL-2026-04-22)
auto_register_ed25519_key only gated on authorized_keys.is_empty() — it did NOT require that the pubkey actually derives to the target address. For EOAs this was implicitly safe (addr = blake3(pubkey)[..20]). For pockets, whose address is derived from (parent, label) and cannot satisfy pubkey-derivation, any attacker could plant their own key on a victim's pocket via SetPolicyTx / AddKeyTx / SetRecoveryTx, then spend from the pocket in a second tx — verify_smart_transfer checks the pocket's own config before falling back to the parent. Same class as GHSA-9chc-gjfr-6hrq, more severe — grants arbitrary key authorization rather than bypassing a limit. - auto_register_ed25519_key early-returns unless Address::from_pubkey(pub_key) == *addr. Pockets now refuse key injection; the is_empty() gate stays. - auto_register_key (pub, RPC relay path) rejects pocket addresses outright as defense in depth. Regression tests in smart_account_pocket_key_injection.rs cover SetPolicy, AddKey, SetRecovery, the full drain chain, and direct auto_register_key pocket rejection (5/5 pass). Found by internal review during the 5-domain security audit that followed the GHSA-9chc-gjfr-6hrq remediation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent afc34a0 commit 826e2a2

4 files changed

Lines changed: 326 additions & 0 deletions

File tree

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2610,6 +2610,19 @@ impl StateEngine {
26102610
/// Called when a legacy account sends its first transaction after SmartAccount creation.
26112611
fn auto_register_ed25519_key(&mut self, addr: &Address, pub_key: &[u8; 32], round: u64) {
26122612
use crate::tx::smart_account::{AuthorizedKey, KeyType};
2613+
2614+
// Only auto-register a key that cryptographically belongs to this
2615+
// address (i.e. blake3(pub_key)[..20] == addr). Pockets and any other
2616+
// non-EOA addresses do NOT derive from any keypair — auto-registering
2617+
// an attacker-supplied key on them would grant that key authorization
2618+
// over the pocket's balance (verify_smart_transfer checks the pocket's
2619+
// own config before falling back to the parent). Pockets inherit
2620+
// authorization from their parent via `pocket_to_parent`, not via
2621+
// their own authorized_keys list, which must stay empty.
2622+
if Address::from_pubkey(pub_key) != *addr {
2623+
return;
2624+
}
2625+
26132626
self.ensure_smart_account_at_round(addr, round);
26142627
if let Some(config) = self.smart_accounts.get_mut(addr) {
26152628
if config.authorized_keys.is_empty() {
@@ -2866,6 +2879,18 @@ impl StateEngine {
28662879
/// Used by the relay endpoint. Creates the SmartAccount if it doesn't exist.
28672880
pub fn auto_register_key(&mut self, addr: &Address, key_type: crate::tx::smart_account::KeyType, pubkey: Vec<u8>) -> Result<(), CoinError> {
28682881
use crate::tx::smart_account::AuthorizedKey;
2882+
2883+
// Pockets must never have their own authorized_keys — they inherit
2884+
// from their parent via pocket_to_parent. Allowing a key to be
2885+
// planted here would let the caller spend from the pocket, because
2886+
// verify_smart_transfer checks the pocket's own config before the
2887+
// parent's.
2888+
if self.pocket_to_parent.contains_key(addr) {
2889+
return Err(CoinError::ValidationError(
2890+
"cannot register a key directly on a pocket address — pockets inherit from their parent".into(),
2891+
));
2892+
}
2893+
28692894
self.ensure_smart_account(addr);
28702895
let config = self.smart_accounts.get_mut(addr)
28712896
.ok_or_else(|| CoinError::ValidationError("smart account not found".into()))?;
Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
//! Regression tests for the pocket key-injection attack chain.
2+
//!
3+
//! A pocket's address is derived from `(parent_address, label)` and does NOT
4+
//! derive from any keypair — so `Address::from_pubkey(pub_key) == pocket_addr`
5+
//! is impossible. Before the fix, `auto_register_ed25519_key` only guarded on
6+
//! `authorized_keys.is_empty()`, which meant any attacker could submit a
7+
//! policy/key/recovery tx targeting a pocket and plant their own key on the
8+
//! pocket's SmartAccountConfig. Because `verify_smart_transfer` checks the
9+
//! pocket's own config before falling back to the parent, the planted key
10+
//! then authorized SmartTransferTx → pocket drain.
11+
//!
12+
//! These tests replay each link in the attack chain and assert the
13+
//! authorization check rejects it.
14+
15+
use ultradag_coin::address::{Address, SecretKey, Signature};
16+
use ultradag_coin::state::StateEngine;
17+
use ultradag_coin::tx::name_registry::derive_pocket_address;
18+
use ultradag_coin::tx::smart_account::*;
19+
20+
fn setup_victim_with_pocket(
21+
alice: &SecretKey, pocket_label: &str, pocket_balance: u64,
22+
) -> (StateEngine, Address) {
23+
let mut engine = StateEngine::new_with_genesis();
24+
engine.faucet_credit(&alice.address(), 20_000_000_000).unwrap();
25+
engine.ensure_smart_account(&alice.address());
26+
27+
// Seed Alice's key so auto-registration on the parent doesn't mask the test.
28+
let alice_pub = alice.verifying_key().to_bytes();
29+
let alice_kid = AuthorizedKey::compute_key_id(KeyType::Ed25519, &alice_pub);
30+
let cfg = engine.smart_account_mut_for_test(&alice.address()).unwrap();
31+
if !cfg.authorized_keys.iter().any(|k| k.key_id == alice_kid) {
32+
cfg.authorized_keys.push(AuthorizedKey {
33+
key_id: alice_kid,
34+
key_type: KeyType::Ed25519,
35+
pubkey: alice_pub.to_vec(),
36+
label: "owner".to_string(),
37+
daily_limit: None,
38+
daily_spent: (0, 0),
39+
});
40+
}
41+
42+
// Alice creates a pocket and funds it.
43+
let pocket_op = SmartOpTx {
44+
from: alice.address(),
45+
operation: SmartOpType::CreatePocket { label: pocket_label.to_string() },
46+
fee: 0, nonce: 0,
47+
signing_key_id: [0u8; 8],
48+
signature: vec![],
49+
webauthn: None,
50+
p256_pubkey: None,
51+
};
52+
engine.apply_smart_op_tx(&pocket_op, 100).unwrap();
53+
let pocket = derive_pocket_address(&alice.address(), pocket_label);
54+
engine.credit(&pocket, pocket_balance).unwrap();
55+
56+
(engine, pocket)
57+
}
58+
59+
#[test]
60+
fn set_policy_cannot_plant_attacker_key_on_pocket() {
61+
let alice = SecretKey::from_bytes([0xA1; 32]);
62+
let mallory = SecretKey::from_bytes([0xEE; 32]);
63+
let (mut engine, pocket) = setup_victim_with_pocket(&alice, "savings", 5_000_000_000);
64+
65+
// Mallory forges a SetPolicyTx targeting Alice's pocket with Mallory's own key.
66+
let mallory_pub = mallory.verifying_key().to_bytes();
67+
let mut tx = SetPolicyTx {
68+
from: pocket,
69+
instant_limit: u64::MAX,
70+
vault_threshold: 0,
71+
vault_delay_rounds: 0,
72+
whitelisted_recipients: vec![],
73+
daily_limit: None,
74+
fee: 0, nonce: 0,
75+
pub_key: mallory_pub,
76+
signature: Signature([0u8; 64]),
77+
};
78+
tx.signature = mallory.sign(&tx.signable_bytes());
79+
80+
let err = engine.apply_set_policy_tx(&tx, 100).unwrap_err();
81+
assert!(
82+
err.to_string().contains("not authorized"),
83+
"expected authorization failure, got: {err}"
84+
);
85+
86+
// Pocket's authorized_keys must remain empty.
87+
if let Some(cfg) = engine.smart_account(&pocket) {
88+
assert!(
89+
cfg.authorized_keys.is_empty(),
90+
"pocket must never accept an auto-registered key"
91+
);
92+
}
93+
}
94+
95+
#[test]
96+
fn add_key_cannot_plant_attacker_key_on_pocket() {
97+
let alice = SecretKey::from_bytes([0xA2; 32]);
98+
let mallory = SecretKey::from_bytes([0xED; 32]);
99+
let (mut engine, pocket) = setup_victim_with_pocket(&alice, "savings", 5_000_000_000);
100+
101+
let mallory_pub = mallory.verifying_key().to_bytes();
102+
let mallory_kid = AuthorizedKey::compute_key_id(KeyType::Ed25519, &mallory_pub);
103+
let mut tx = AddKeyTx {
104+
from: pocket,
105+
new_key: AuthorizedKey {
106+
key_id: mallory_kid,
107+
key_type: KeyType::Ed25519,
108+
pubkey: mallory_pub.to_vec(),
109+
label: "attacker".to_string(),
110+
daily_limit: None,
111+
daily_spent: (0, 0),
112+
},
113+
fee: 0, nonce: 0,
114+
pub_key: mallory_pub,
115+
signature: Signature([0u8; 64]),
116+
};
117+
tx.signature = mallory.sign(&tx.signable_bytes());
118+
119+
let err = engine.apply_add_key_tx(&tx).unwrap_err();
120+
assert!(
121+
err.to_string().contains("not authorized"),
122+
"expected authorization failure, got: {err}"
123+
);
124+
125+
if let Some(cfg) = engine.smart_account(&pocket) {
126+
assert!(cfg.authorized_keys.is_empty());
127+
}
128+
}
129+
130+
#[test]
131+
fn set_recovery_cannot_plant_attacker_key_on_pocket() {
132+
let alice = SecretKey::from_bytes([0xA3; 32]);
133+
let mallory = SecretKey::from_bytes([0xEC; 32]);
134+
let (mut engine, pocket) = setup_victim_with_pocket(&alice, "savings", 5_000_000_000);
135+
136+
let mallory_pub = mallory.verifying_key().to_bytes();
137+
let mut tx = SetRecoveryTx {
138+
from: pocket,
139+
guardians: vec![mallory.address()],
140+
threshold: 1,
141+
delay_rounds: 100,
142+
fee: 0, nonce: 0,
143+
pub_key: mallory_pub,
144+
signature: Signature([0u8; 64]),
145+
};
146+
tx.signature = mallory.sign(&tx.signable_bytes());
147+
148+
let err = engine.apply_set_recovery_tx(&tx).unwrap_err();
149+
assert!(
150+
err.to_string().contains("not authorized"),
151+
"expected authorization failure, got: {err}"
152+
);
153+
154+
if let Some(cfg) = engine.smart_account(&pocket) {
155+
assert!(cfg.authorized_keys.is_empty());
156+
}
157+
}
158+
159+
#[test]
160+
fn full_drain_attack_chain_fails() {
161+
let alice = SecretKey::from_bytes([0xA4; 32]);
162+
let mallory = SecretKey::from_bytes([0xEB; 32]);
163+
let (mut engine, pocket) = setup_victim_with_pocket(&alice, "savings", 5_000_000_000);
164+
let initial_pocket_balance = engine.balance(&pocket);
165+
166+
// Step 1: Mallory tries to plant her key on the pocket via SetPolicyTx.
167+
// (Before the fix, this would succeed and register mallory_pub on the pocket.)
168+
let mallory_pub = mallory.verifying_key().to_bytes();
169+
let mut set_policy = SetPolicyTx {
170+
from: pocket,
171+
instant_limit: u64::MAX,
172+
vault_threshold: 0,
173+
vault_delay_rounds: 0,
174+
whitelisted_recipients: vec![],
175+
daily_limit: None,
176+
fee: 0, nonce: 0,
177+
pub_key: mallory_pub,
178+
signature: Signature([0u8; 64]),
179+
};
180+
set_policy.signature = mallory.sign(&set_policy.signable_bytes());
181+
let _ = engine.apply_set_policy_tx(&set_policy, 100);
182+
183+
// Step 2: Mallory attempts the drain. Even if step 1 had silently no-op'd
184+
// on the authorization side, a subsequent SmartTransferTx must fail at
185+
// verify_smart_transfer because Mallory's key is neither on the pocket
186+
// (the fix) nor on Alice's parent config.
187+
let mallory_kid = AuthorizedKey::compute_key_id(KeyType::Ed25519, &mallory_pub);
188+
let mut transfer = SmartTransferTx {
189+
from: pocket,
190+
to: mallory.address(),
191+
amount: initial_pocket_balance,
192+
fee: 0, nonce: 0,
193+
signing_key_id: mallory_kid,
194+
signature: vec![],
195+
memo: None,
196+
webauthn: None,
197+
};
198+
transfer.signature = mallory.sign(&transfer.signable_bytes()).0.to_vec();
199+
200+
assert!(
201+
!engine.verify_smart_transfer(&transfer),
202+
"Mallory's signature must not verify against the pocket or its parent"
203+
);
204+
205+
// Pocket balance must be untouched.
206+
assert_eq!(engine.balance(&pocket), initial_pocket_balance);
207+
}
208+
209+
#[test]
210+
fn auto_register_key_rejects_pocket_address() {
211+
let alice = SecretKey::from_bytes([0xA5; 32]);
212+
let mallory = SecretKey::from_bytes([0xEA; 32]);
213+
let (mut engine, pocket) = setup_victim_with_pocket(&alice, "savings", 1_000_000_000);
214+
215+
// The RPC relay-endpoint path. Even though this path is currently
216+
// called with a p256-derived address in production, the function must
217+
// refuse to register a key directly on a pocket as defense in depth.
218+
let mallory_pub = mallory.verifying_key().to_bytes();
219+
let err = engine
220+
.auto_register_key(&pocket, KeyType::Ed25519, mallory_pub.to_vec())
221+
.unwrap_err();
222+
assert!(
223+
err.to_string().contains("pocket"),
224+
"expected pocket-specific rejection, got: {err}"
225+
);
226+
}
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
# INTERNAL-2026-04-22 — Pocket key-injection enables pocket drain
2+
3+
**Severity:** Critical
4+
**Component:** `ultradag-coin``StateEngine` key auto-registration
5+
**Disclosure:** Internal review (not externally reported; no bounty payout)
6+
**Status:** Fixed
7+
8+
## Summary
9+
10+
`auto_register_ed25519_key` gated key registration on
11+
`authorized_keys.is_empty()` but did NOT require that the supplied pubkey
12+
actually derives to the target address. For regular EOAs (`addr =
13+
blake3(pubkey)[..20]`) this was implicitly safe because only the legitimate
14+
keyholder can produce a pubkey that derives to the address. Pockets broke
15+
the assumption: a pocket's address is computed from `(parent_address,
16+
label)` and no pubkey can satisfy the derivation, so the implicit trust was
17+
never crosschecked.
18+
19+
Any attacker could submit a policy, key, or recovery tx targeting a
20+
victim's pocket with their own pubkey as the signer. The auto-registration
21+
path would plant the attacker's key on the pocket's `SmartAccountConfig`;
22+
the subsequent authorization check (`is_key_authorized`) would then find
23+
the just-added key and pass. Because `verify_smart_transfer` checks the
24+
pocket's own config before falling back to the parent, the attacker's
25+
planted key then authorized `SmartTransferTx` from the pocket — draining
26+
the balance in a second tx.
27+
28+
Sister path: `auto_register_key` (pub; used by the relay endpoint)
29+
unconditionally pushed a key to any target. The one live caller passes a
30+
pubkey-derived address so there was no exploitable site today, but the
31+
function itself offered no defense-in-depth.
32+
33+
## Impact
34+
35+
Same attack class as GHSA-9chc-gjfr-6hrq but more severe — grants
36+
arbitrary attacker-key authorization on a victim's pocket rather than
37+
merely bypassing a limit. Every pocket on the network with any balance
38+
was drainable by any funded address in two txs (one to plant the key,
39+
one to spend).
40+
41+
## Root cause
42+
43+
`auto_register_ed25519_key` did not enforce the invariant that a key can
44+
only be auto-registered on an address that cryptographically derives from
45+
it. The guard `authorized_keys.is_empty()` only prevented overwriting an
46+
already-owned account — not hostile first-time registration on a non-EOA
47+
address.
48+
49+
## Fix
50+
51+
- `auto_register_ed25519_key`: early-return unless `Address::from_pubkey(pub_key)
52+
== *addr`. Non-EOA addresses (pockets today, any future non-derived
53+
address type) now refuse arbitrary key injection. The existing
54+
`is_empty()` gate stays in place.
55+
- `auto_register_key` (pub): rejects pocket addresses outright with a
56+
ValidationError. Defense in depth — ensures the RPC relay path can't be
57+
weaponized if a future code path passes a pocket address by accident.
58+
59+
The fix restores the correct invariant: **pockets never hold their own
60+
authorized keys**. Authorization for pocket-originated transfers flows
61+
exclusively through `pocket_to_parent` → parent config →
62+
`verify_smart_transfer` fallback.
63+
64+
Five regression tests in
65+
`crates/ultradag-coin/tests/smart_account_pocket_key_injection.rs` cover
66+
each attack surface (`SetPolicyTx`, `AddKeyTx`, `SetRecoveryTx`, full
67+
drain chain, and direct `auto_register_key` rejection).
68+
69+
## Credits
70+
71+
Internal review during a 5-domain parallel audit (consensus / state /
72+
network / economic / crypto). Found by the state-engine review which
73+
flagged the unguarded pocket surface immediately after the
74+
GHSA-9chc-gjfr-6hrq remediation.

docs/security/advisories/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ This directory contains published security advisories for vulnerabilities that h
55
## Published Advisories
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.
8+
- [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.
89

910
## Disclosure Timeline
1011

0 commit comments

Comments
 (0)