Skip to content

Commit b44f7b4

Browse files
fix(wallet): refuse a spend whose input coin amounts do not sum in a u64
Found while delete-probing the guards: dig-wallet-backend 0.16 accumulates the spent coins' amounts with an unchecked `+=` (client/verify.rs:153). Those amounts come from an unsigned skeleton a dapp supplies, so they are attacker-chosen and need not name coins that exist — an unsummable input total therefore PANICS in a debug build and WRAPS in a release build, after which the wrapped figure is what value conservation is checked against. Refusing it here as PolicyIndeterminate makes that unreachable. Also merge the rolling-cap's two overflow checks into the one that is reachable: the window's own total could only overflow if two recorded charges summed past u64::MAX, which the projection check already prevents from ever being recorded, so that half could never fire and no test could hold it. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent d0e900e commit b44f7b4

3 files changed

Lines changed: 175 additions & 10 deletions

File tree

probe.sh

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
#!/usr/bin/env bash
2+
# Delete-probe each guard: neutralize it, run the suite, restore. Red = the guard is load-bearing.
3+
set -u
4+
cd "$(dirname "$0")"
5+
6+
probe() {
7+
local name="$1" file="$2" from="$3" to="$4"
8+
cp "$file" /tmp/probe.bak
9+
python3 - "$file" "$from" "$to" <<'PY'
10+
import io,sys
11+
p,a,b=sys.argv[1],sys.argv[2],sys.argv[3]
12+
s=io.open(p,encoding='utf-8').read()
13+
if a not in s:
14+
print("PATTERN-MISS"); sys.exit(3)
15+
io.open(p,'w',encoding='utf-8',newline='').write(s.replace(a,b,1))
16+
PY
17+
if [ $? -ne 0 ]; then echo "$name :: PATTERN-MISS"; cp /tmp/probe.bak "$file"; return; fi
18+
out=$(cargo test 2>&1)
19+
failed=$(echo "$out" | sed -n '/^failures:$/,$p' | grep -oE "^ [a-z_][a-z_0-9:]*$" | tr -d ' ' | sort -u | tr '\n' ' ')
20+
if echo "$out" | grep -q "test result: FAILED"; then
21+
verdict="RED -> $failed"
22+
elif ! echo "$out" | grep -q "test result:"; then
23+
verdict="INCONCLUSIVE (the mutation itself did not compile)"
24+
else
25+
verdict="GREEN - VACUOUS"
26+
fi
27+
echo "$name :: $verdict"
28+
cp /tmp/probe.bak "$file"
29+
}
30+
31+
E=src/wallet/enforcer.rs
32+
S=src/wallet/summary.rs
33+
A=src/wallet/approval.rs
34+
35+
probe "G7 an undeclared intent escalates" $E 'self.auto_send.configured_limits(op_class)' 'self.auto_send.configured_limits(match op_class {
36+
SpendOpClass::Undeclared => SpendOpClass::Tip,
37+
declared => declared,
38+
})'
39+
40+
probe "G2 custody total is CHECKED, not saturating" $S 'let native_total_mojos = summary.checked_native_total_mojos()?;' 'let native_total_mojos = summary.native_total_mojos();'
41+
42+
probe "G19 input coin amounts must sum in a u64" $S 'coin_spends
43+
.iter()
44+
.try_fold(0u64, |sum, spend| sum.checked_add(spend.coin.amount))
45+
.ok_or_else(|| {' 'coin_spends
46+
.iter()
47+
.try_fold(0u64, |sum, spend| Some(sum.wrapping_add(spend.coin.amount)))
48+
.ok_or_else(|| {'
49+
50+
probe "G13 the rolling projection is checked, not wrapped" $E '.try_fold(total, |sum, record| sum.checked_add(record.mojos))' '.try_fold(total, |sum, record| Some(sum.wrapping_add(record.mojos)))'

src/wallet/enforcer.rs

Lines changed: 88 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -322,18 +322,22 @@ impl PolicyAuthorizer {
322322

323323
recent.retain(|record| record.at_unix.saturating_add(self.auto_send.period_seconds) > now);
324324

325-
let already: u64 = recent.iter().try_fold(0u64, |sum, record| {
326-
sum.checked_add(record.mojos).ok_or_else(|| {
325+
// ONE checked accumulation, starting from this spend, rather than summing the window and then
326+
// adding to it. The two-step form had an unreachable half: the window's own total can only
327+
// overflow if two recorded charges sum past `u64::MAX`, which the projection check below
328+
// already prevents from ever being recorded — so that guard could never fire, and a guard that
329+
// cannot fire is a guard no test can hold. Folded together, the single remaining check IS
330+
// reachable (a `u64::MAX` charge under a `u64::MAX` cap, then one mojo more).
331+
let projected = recent
332+
.iter()
333+
.try_fold(total, |sum, record| sum.checked_add(record.mojos))
334+
.ok_or_else(|| {
327335
AccountError::PolicyIndeterminate(
328-
"the auto-send ledger total overflows u64".to_string(),
336+
"this spend plus the rolling period total overflows u64, so the cap cannot be \
337+
evaluated"
338+
.to_string(),
329339
)
330-
})
331-
})?;
332-
let projected = already.checked_add(total).ok_or_else(|| {
333-
AccountError::PolicyIndeterminate(
334-
"this spend plus the period total overflows u64".to_string(),
335-
)
336-
})?;
340+
})?;
337341

338342
if projected > self.auto_send.period_cap_mojos {
339343
return Ok(CapVerdict::OverCap);
@@ -455,6 +459,26 @@ mod tests {
455459
ctx.take()
456460
}
457461

462+
/// Two wallet-owned input coins, each paid straight out to a hinted third party.
463+
///
464+
/// A multi-coin fixture is the only shape whose INPUT total can exceed a single coin's `u64`, so it
465+
/// is the only one that can exercise the input-summability guard.
466+
fn two_coin_spend(first: u64, second: u64) -> Vec<CoinSpend> {
467+
let recipient = third_party().puzzle_hash();
468+
let mut ctx = SpendContext::new();
469+
for (parent, amount) in [([1u8; 32], first), ([2u8; 32], second)] {
470+
let hint = ctx.hint(recipient).unwrap();
471+
StandardLayer::new(spender().public_key())
472+
.spend(
473+
&mut ctx,
474+
Coin::new(Bytes32::new(parent), spender().puzzle_hash(), amount),
475+
Conditions::new().create_coin(recipient, amount, hint),
476+
)
477+
.unwrap();
478+
}
479+
ctx.take()
480+
}
481+
458482
/// A spend paying `amount` to a third party with no fee — the workhorse fixture.
459483
fn pays_third_party(amount: u64) -> Vec<CoinSpend> {
460484
spend_paying(&[(third_party().puzzle_hash(), amount)], 0)
@@ -1411,4 +1435,58 @@ mod tests {
14111435
"the signer must refuse un-hinted value leaving the wallet: {err}"
14121436
);
14131437
}
1438+
1439+
/// A coin-spend set whose INPUT amounts do not sum in a `u64` is refused rather than judged.
1440+
///
1441+
/// This is the guard over `dig-wallet-backend` 0.16's unchecked input accumulation
1442+
/// (`client/verify.rs:153`): without it this very fixture panics in a debug build and, in a release
1443+
/// build, has its input total WRAP — after which the wrapped figure is what value conservation is
1444+
/// checked against. The amounts come from an unsigned skeleton a dapp supplies, so they are
1445+
/// attacker-chosen and need not name coins that exist.
1446+
///
1447+
/// The truthful control matters here more than usual: the same two-coin shape at halved amounts
1448+
/// sums fine and is judged normally, so what follows is the SUM being unrepresentable and not the
1449+
/// gate refusing multi-coin spends.
1450+
#[test]
1451+
fn a_spend_whose_input_amounts_do_not_sum_in_a_u64_is_refused_rather_than_wrapped() {
1452+
let gate = gate_with(hot_custody(), permissive_auto_send());
1453+
1454+
let err =
1455+
refusal(gate.authorize_op(&two_coin_spend(u64::MAX, u64::MAX), SpendOpClass::Tip));
1456+
assert!(
1457+
matches!(&err, AccountError::PolicyIndeterminate(m) if m.contains("sum in a u64")),
1458+
"an unsummable input total must be indeterminate, and say so: {err}"
1459+
);
1460+
1461+
// Control: two coins whose amounts DO sum are judged on their merits.
1462+
let judged = gate.authorize_op(&two_coin_spend(1_000, 2_000), SpendOpClass::Tip);
1463+
assert!(
1464+
!matches!(&judged, Err(AccountError::PolicyIndeterminate(_))),
1465+
"the same two-coin shape must be judgeable when its total is representable"
1466+
);
1467+
}
1468+
1469+
/// The rolling ledger's projection is checked, so a charge that would push the window total past
1470+
/// `u64::MAX` is indeterminate rather than wrapped into a small, comfortably-under-cap number.
1471+
///
1472+
/// Reachable only under a `u64::MAX` cap, which is why the fixture uses one: charge the whole
1473+
/// `u64::MAX` allowance, then ask for one mojo more.
1474+
#[test]
1475+
fn a_charge_that_would_overflow_the_rolling_total_is_indeterminate_not_wrapped() {
1476+
let gate = gate_with(
1477+
CustodyPolicy::Hot(HotWallet {
1478+
auto_send_limit: u64::MAX,
1479+
}),
1480+
permissive_auto_send(),
1481+
);
1482+
1483+
approval(gate.authorize_op(&pays_third_party(u64::MAX), SpendOpClass::Tip));
1484+
assert_eq!(ledger_total(&gate), u64::MAX);
1485+
1486+
let err = refusal(gate.authorize_op(&pays_third_party(1), SpendOpClass::Tip));
1487+
assert!(
1488+
matches!(&err, AccountError::PolicyIndeterminate(m) if m.contains("overflows u64")),
1489+
"the projection must refuse rather than wrap to 0: {err}"
1490+
);
1491+
}
14141492
}

src/wallet/summary.rs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,29 @@ impl SpendSummary {
177177
/// This is the crate's ONE call into [`derive_summary`]: value conservation, quote-form delegated
178178
/// puzzles and the sole-`AGG_SIG_ME` rule are checked here, so a spend the driver cannot fully account
179179
/// for is refused before any custody decision — and before any signature — exists.
180+
///
181+
/// # Why the input amounts are summed first
182+
///
183+
/// `dig-wallet-backend` 0.16's `derive_summary` accumulates the spent coins' amounts with an unchecked
184+
/// `+=` (`client/verify.rs:153`), so a coin-spend set whose INPUT amounts do not sum in a `u64` panics
185+
/// in a debug build and WRAPS in a release build — and a wrapped input total is compared against the
186+
/// outputs to decide value conservation. The amounts come from an unsigned skeleton, which a dapp
187+
/// supplies, so they are attacker-chosen and need not correspond to coins that exist. Refusing an
188+
/// unsummable input total here keeps that reachable from nowhere: the answer is
189+
/// [`PolicyIndeterminate`](AccountError::PolicyIndeterminate), because a spend whose inputs cannot be
190+
/// totalled is not forbidden — it simply cannot be judged.
180191
fn derive_verified(coin_spends: &[CoinSpend]) -> Result<TransactionSummary> {
192+
coin_spends
193+
.iter()
194+
.try_fold(0u64, |sum, spend| sum.checked_add(spend.coin.amount))
195+
.ok_or_else(|| {
196+
AccountError::PolicyIndeterminate(
197+
"the spent coins' amounts do not sum in a u64, so this spend's value cannot be \
198+
accounted for"
199+
.to_string(),
200+
)
201+
})?;
202+
181203
derive_summary(coin_spends)
182204
.map_err(|e| AccountError::Spend(format!("cannot derive spend summary: {e}")))
183205
}
@@ -204,6 +226,21 @@ impl DerivedSpend {
204226
/// Fail-closed at each step: an unaccountable spend is refused by the verify gate, and one whose
205227
/// native amounts cannot be summed in a `u64` is [`PolicyIndeterminate`](AccountError::PolicyIndeterminate)
206228
/// rather than clamped to `u64::MAX` and then tiered as though the clamp were its value.
229+
///
230+
/// # The checked sum here cannot currently fail, and is kept anyway
231+
///
232+
/// [`derive_verified`] refuses a spend whose INPUT amounts do not sum in a `u64`, and the verify
233+
/// gate then requires value conservation, so the outputs plus the fee equal a figure that already
234+
/// fits — the native total is bounded before this line runs. A delete-probe confirms it: swapping
235+
/// [`checked_native_total_mojos`](SpendSummary::checked_native_total_mojos) for the saturating
236+
/// accessor turns no test red.
237+
///
238+
/// It stays because the bound rests on a DEPENDENCY's conservation check rather than on anything
239+
/// this crate can see. If that check is ever relaxed, narrowed, or bypassed for a new spend shape,
240+
/// the honest answer to "what is this spend worth" must become a refusal — never `u64::MAX` quietly
241+
/// standing in for a number nobody computed. This is the one guard in the custody path that is
242+
/// deliberately retained without a test able to hold it, and it is called out rather than left for
243+
/// a reader to discover.
207244
pub(crate) fn derive(coin_spends: &[CoinSpend], policy: &CustodyPolicy) -> Result<Self> {
208245
let verified = derive_verified(coin_spends)?;
209246
// Tiered `Confirm` first — the stricter of the two hot tiers, so a bug that skipped the

0 commit comments

Comments
 (0)