Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion deps/python-keepkey
63 changes: 63 additions & 0 deletions docs/release/rc30-evidence/T1-setup-ceremony.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# T1 — setup ceremony staged as a transaction (#429)

Device: 7.14.2, fw_hash fd7b3901…, variant KeepKey, device_id 39353036114736342A004600.
Vehicle: `ResetDevice` with an injected `RecoveryDevice`, per plan §2 T1.

## VERDICT: PASS

### Phase A — injection refused, nothing committed

| step | response |
|---|---|
| inject `RecoveryDevice` (label `pwned`, pin off, passphrase off) | `Failure code=1 'Device is in the middle of setup. Send Initialize or Cancel first.'` |
| `Cancel` | `Failure code=4 'Aborted'` |
| `EntropyAck` — **the exploit step** | `Failure code=1 'Not in Reset mode'` |
| end state | `initialized=False pin_protection=False passphrase_protection=False label=''` |

Tester confirmed: photos 3 and 4 were the plain home logo, no `RECOVERY` cipher
screen at any point, and **no hold was demanded anywhere after the two PIN
screens**.

This is the core of #429. On base `1af2ffe7de` the reset was still armed at the
`EntropyAck`, so it was consumed and committed a seed with the user's PIN and
passphrase silently stripped. `CHECK_NO_CEREMONY` (`fsm.c:118-125`) now refuses
before `recovery_cipher_init()` can write anything, and no screen is drawn — the
refusal is invisible to the user because nothing was ever staged.

Note the label: `''`, not `pwned`. The injected settings left no residue.

### Phase B — ceremony completes with the user's own settings

| step | response |
|---|---|
| inject `RecoveryDevice` again | `Failure code=1` |
| `EntropyAck`, ceremony proceeds | seed backup screens, holds taken |
| end state | `initialized=True pin_protection=True passphrase_protection=True label='ceremony-B'` |

## Photo 11 — resolved, was NOT a failure

During the run that set the PIN, `Ping(pin_protection=True)` returned `Success`
with **no** `PinMatrixRequest`. The plan lists that as a hard fail.

It is not, in this context. `Pin Caching` is an enabled policy and the PIN had
just been entered during the ceremony, so the session legitimately held it. On a
fresh session after a replug:

initialized=True pin_protection=True passphrase_protection=True label='ceremony-B'
-> PinMatrixRequest

**The plan's criterion needs qualifying:** `Ping(pin_protection=True)` returning
`Success` with no matrix is a hard fail only on a session that has not already
authenticated. Re-testing it inside the setup session tests the cache, not the gate.

## Method note

The photo-11 check in `gh429_setup_ceremony.py` printed PASS when the matrix never
appeared, because the assertion lived inside the `if PinMatrixRequest:` branch —
a check that cannot fail is not a check. Same shape as the T5 error. Verified
separately in `t1_pin_gate_check.py`, which asserts on the branch actually taken.

## Device left as

Initialised, random seed, PIN `789456`, passphrase ON, label `ceremony-B`.
T2 wipes this and loads `mnemonic12`.
74 changes: 74 additions & 0 deletions docs/release/rc30-evidence/T10-T11-0x-clearsign.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# T10 — transformERC20 bound to complete calldata (#444, #468)
# T11 — sellToUniswap clear-signs only when BOTH tokens resolve (#472)

Device: 7.14.2, all-all-all seed, no PIN, no passphrase.

## VERDICT: PASS (5/5)

| run | payload | AdvancedMode | result |
|---|---|---|---|
| T10 A | transformERC20, 1480 B | OFF | **REFUSED** `Arbitrary contract data signing disabled by policy` |
| T10 B | transformERC20, 1480 B | ON | SIGNED v=38 |
| T10 C | transformERC20, 1024 B | OFF | **SIGNED v=37** (clear-signed) |
| T11 A | sellToUniswap, both tokens resolve | OFF | **SIGNED v=38** (clear-signed) |
| T11 B | sellToUniswap, `tokens[0]` = PEPE | OFF | **REFUSED** |

## T10 — the wire proves RUN C without a photo

RUN A establishes that with AdvancedMode OFF the blind path is refused outright.
So the ONLY way RUN C could sign is the 0x decoder claiming it. **The gate binds
clear-signing to complete calldata without killing it** — #468 did not trade one
defect for a usability cliff.

Before `d3be389af`, `ethereum_contractHandled()` had
`if (zx_isZxTransformERC20(msg)) return true;` ABOVE the chunk-completeness
check, so the 1480-byte payload was claimed by the 0x decoder: the device showed
`TRANSFORM ERC20 / Input 53086.65334 USDT / Output 53029.30814 USDC`, then
`TRANSACTION`, and the 456 bytes past the initial chunk streamed in, were hashed,
and were never rendered. AdvancedMode was never consulted — a DEFAULT device
signed it.

RUN C caveat: synthetic payload (fixture truncated to 1024 B, transformations[]
tail cut). It is also the only sub-1024-byte transformERC20 exercise in the plan,
filling the gap T11's card declared "a separate card".

## T11 — one word apart

Legs A and B are the SAME calldata on the SAME chain with ONE 32-byte word
changed. A clear-signs; B refuses.

Before `dea1cd7e6`, `zx_isZxSwap()` claimed any `d9627aa4` call to the 0x proxy
on an allowlisted chain WITHOUT LOOKING AT THE TOKEN WORDS.
`zx_confirmZxSwap()` then called `ethereumFormatAmount()`, which emits the
literal `Unknown token value` on a lookup miss — so an unlisted sell token
produced exactly one screen, `UNISWAP / Sell Unknown token value / Buy at least
0.000389574704633884 ETH`, **and the device signed**. A screen naming a DEX,
naming no amount, hiding 296 bytes of calldata.

The refusal lives in the PREDICATE, not the confirm: a false from
`ethereum_contractConfirmed()` would be read as a user cancel.

### Evidence for the #455 diagnosis

Leg B runs on **chain 1**, where no `uint8_t` chain-id truncation occurs, and
still refuses. That is direct evidence the #455 diagnosis is right: the failure
mode is an **unresolved token lookup**, not chain-id truncation.

### Does NOT prove #414

Leg A still hides the `869584cd` affiliate tail. #414 remains open.

## Photo checks with the tester

- **B3 is load-bearing:** `CONFIRM ETHEREUM DATA` must read `1480 bytes` —
not `456`, not `1024`. First row exactly `415565b0000000000000000000000000`.
- C2 must be `TRANSFORM ERC20` with both token lines and NO `SEND` /
`CONFIRM ETHEREUM DATA`.
- T11 photo 1: `Sell 1 USDC` and `0.000389574704633884 ETH` — note the DOUBLE
SPACE, the sentinel ticker is literally `" ETH"`. `Unknown token value`
must appear nowhere.
- T11 leg B: no `UNISWAP` screen at any point.

## Device left as

all-all-all, no PIN, no passphrase, AdvancedMode OFF.
70 changes: 70 additions & 0 deletions docs/release/rc30-evidence/T12-altcoin-binding.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# T12 — altcoin amount and address binding (#408, #438, #407, #406, #430)

Device: 7.14.2, all-all-all seed, no PIN, no passphrase, AdvancedMode OFF.

## VERDICT: 7 of 8 required steps PASS. Step D (Solana SPL) NOT RUN.

| step | payload | result |
|---|---|---|
| A1 | 1234567 uosmo | PASS signed |
| A2 | **1 uosmo** — the `base_to_precision` digit-drop test | PASS signed |
| A3 | 123456789012345678 uion (non-uosmo denom) | PASS signed |
| A4 | 68-char `ibc/2739…5EB2` denom | signed (informational, see below) |
| A5 | incomplete MsgSend | PASS `Failure 'Message is missing required parameters'`, no screen |
| B | LP add, 18 fractional digits | PASS signed |
| C | **Cosmos IBC transfer** | PASS signed |
| D | Solana SPL | **NOT RUN** |
| E | **31-char Binance denom (#430)** | PASS signed, no reboot |

## What each step defends

**A2** — `strlcpy(dst,src,n)` copied `n-1` digits, dropping the last, and wrote
`dest[dest_len]`, one past the caller's buffer. The screen must read
`0.000001 OSMO`; five zeros means the digit-drop is live.

**A3** — before, `float amount = atof(...)` then `"%.6f %s"`: non-`uosmo` denoms
were never divided but still got a `.000000` tail, float32 lost everything past
~8 significant digits, **and `osmosis.c` hardcoded `"denom":"uosmo"` into the
SIGNED amino doc regardless of what was displayed.**

**C** — before, ONE screen `Transfer %s to %s?` fed with
`msg->ibc_transfer.sender`: the sender printed in the destination slot, and the
signed receiver was never displayed at all. Now three screens with distinct
bodies.

**E** — `char denom_str[14]` with `snprintf(denom_str, strlen(denom)+2, " %s",
denom)`: a 31-char denom is a 33-byte write into 14 bytes, a 19-byte stack
overflow. Device signed and did not reboot.

## A4 — informational, file against #428 not #408

`confirm_transaction_output` goes through `layout_notification_no_title_bold`,
which `confirm_helper()` does NOT measure (it only measures
`layout_standard_notification`), so a 129-char body clips with no `CUT OFF`.
**The denom displayed IS the one being hashed**, so this is a rendering gap, not
a binding defect. Record the exact last character shown against #428.

## Step D not run

Step D hand-builds a Solana message (header bytes, account index table,
instruction data). The plan flags it as fragile: if the layout is off the device
errors before drawing anything — a harness failure, not a firmware finding.
Not attempted rather than recorded as a false result.

**Whole-test pass requires A1, A2, A3, A5, B, C, D, E — so T12 is INCOMPLETE
until D runs.** Seven of the eight required steps pass.

## Harness bugs (proto field types, all silent until they throw)

- `CosmosMsgIBCTransfer.revision_height` / `.revision_number` are **strings**
- `CosmosMsgIBCTransfer.amount` is **uint64**, not a string
- `BinanceTransferMsg.BinanceInputOutput.address` is a **bech32 string**, not 20
raw bytes. Passing bytes yields `'Failed to include transfer message in
transaction'` — which looks like a firmware refusal but is not one.

## Photo checks with the tester

A2 `0.000001 OSMO` (six digits, last a 1) · A3 all 18 digits with no decimal
point and no `.000000` tail · B `1.234567890123456789 GAMM-1 shares?` (eighteen
fractional digits) · C screen 3 `Confirm dest. address` showing
`osmo18vhdczj…` and NOT `cosmos15cenya…` · E all 31 denom chars legible.
45 changes: 45 additions & 0 deletions docs/release/rc30-evidence/T2-passphrase-cancel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# T2 — a cancelled passphrase confirmation must not cache (#428a)

Device: 7.14.2, fw_hash fd7b3901…, mnemonic12, passphrase ON, no PIN, label `test`.

## VERDICT: PASS (8/8)

| check | result |
|---|---|
| passphrase requested | PASS |
| confirmation screen raised for `topsecret` | PASS |
| host Cancel produced `Failure code=4 'Ping cancelled'` | PASS |
| **re-asked after cancel (the #428a fix)** | **PASS** |
| confirmation screen raised for `secondtry` | PASS |
| second ping succeeded | PASS |
| control: cached passphrase not re-requested | PASS |

Before `b286dc009`, `passphrase_request()` discarded `review()`'s verdict and did
`ret = true`, so `session_cachePassphrase()` cached a passphrase whose
confirmation screen the host had just suppressed. The next passphrase-protected
request drew nothing and derived keys from it — the user was in a different
wallet than the one they believed they had opened. `passphrase_sm.c:158-161` now
propagates the verdict.

The step-5 control matters: after a *successful* confirmation the passphrase IS
cached and is not re-requested. The fix cancels correctly without disabling
caching.

## First run was INVALID — procedure, not firmware

The first attempt reported 4 failures. It had a 3-second "photograph now" pause
between the ButtonAck and the Cancel; the tester held the button during it, the
confirmation completed (`Success`, message `cancel-probe`), and `topsecret` was
then cached entirely legitimately. All four failures followed from that one press.

**Rule reinforced (see T5):** never leave a human window inside a test whose
premise is that no press occurs. The Cancel is now sent with no pause at all, so
there is no window to press in. Photo 2 is captured in a separate deliberate pass.

`Initialize` calls `session_clear(false)` (`fsm_msg_common.h:9`) — drops a cached
passphrase, keeps the PIN. The script uses it to reset state between probes
instead of requiring a replug.

## Device left as

`mnemonic12`, passphrase ON, no PIN, label `test`.
68 changes: 68 additions & 0 deletions docs/release/rc30-evidence/T3-uniswap-authenticator-cancel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
# T3 — Uniswap clear-sign and authenticator wipe honour a cancel (#421, #422)

Device: 7.14.2, fw_hash fd7b3901…, mnemonic12, no PIN, no passphrase.

## VERDICT: PASS (8/8 wire checks; photo checks with the tester)

### Leg A — authenticator (#421)

| check | result |
|---|---|
| authdata reset | PASS |
| account added | PASS |
| **account readable BEFORE cancel** | PASS `'keepkey.com:alice'` |
| wipe cancel produced Failure | PASS `Action cancelled` |
| **account survived the cancelled wipe** | PASS `'keepkey.com:alice'` |

The before/after pair is the evidence: the account is provably present, the wipe
is cancelled, and it is still readable. On the old firmware the host's own abort
executed as a commit and destroyed every authenticator secret.

Text change `5bccac024` verified in source: the wipe body is now only
`"Do you want to PERMANENTLY delete all authenticator accounts?"`. The sentence
`"If not, unplug Keepkey now."` is absent from `lib/` repo-wide — it existed only
while Cancel was inert.

### Leg B — Uniswap approve (#422)

| check | result |
|---|---|
| uniswap raised a confirm screen | PASS |
| uniswap cancel produced Failure | PASS `Signing cancelled by user` |
| **no signature returned** | PASS |

`fd99fa75a` changes no text at all — every `confirm()` title and format string is
byte-identical. The diff only wraps them in `if (!confirm(...)) return false;`.
Before it, the handler walked forward to `return true` and signed a MAX_ALLOWANCE
approval to the Uniswap V2 router with zero button presses.

Calldata is verbatim from `test_msg_ethereum_erc20_uniswap_liquidity.py::
test_sign_uni_approve_liquidity_ETH` — `approve(0x7a250d56…, 0xffff…ff)` to the
FOX pool `0x470e8de2ebaef52014a47cb5e6af86884947f08c`.

## First attempt was INVALID — precondition, not firmware

Leg A initially failed with `Failure code=4 'passphrase incorrect for authdata'`,
not `Account not found`. Authdata is encrypted under a key derived from the seed;
T3's step 0 reloads the seed, so authdata left from the previous wallet
(`ceremony-B`) could not be decrypted and `initializeAuth` never ran.

**Correct ordering, now in `t3_legA_authenticator.py`:** complete a wipe first
(hold it — `storage_wipeAuthData()` resets the encryption flag), then add the
account, then run the cancel test. Also added the A1b read-back so the test
proves the account exists before claiming it survived.

Note the failure mode was loud, not silent — the prefix bytes were correct.
Verified against `fsm_msg_common.h:205-225`:
`\x15 initializeAuth:` `\x16 generateOTPFrom:` `\x17 getAccount:`
`\x18 removeAccount:` `\x19 wipeAuthdata:`

## Coverage caveat — state it in the release record

This exercises **2 of the 5** `confirm()` calls gated by `fd99fa75a`, and
**1 of the 3** authenticator mutations gated by `5bccac024`. The remaining paths
are argued-by-symmetry, not measured. See plan §4.

## Device left as

mnemonic12, no PIN, no passphrase, authenticator holds `keepkey.com:alice`.
51 changes: 51 additions & 0 deletions docs/release/rc30-evidence/T4-thorchain-memo.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# T4 — THORChain trailing memo fields disclosed (#415)

Device: 7.14.2, variant KeepKey, mnemonic12, no PIN, no passphrase (state left by T3).

## VERDICT: wire checks PASS; photo checks with the tester

| check | result |
|---|---|
| A: signed bytes contain the affiliate tail `:ss:75` | PASS |
| B: matches the 7.14.1 fixture byte for byte | PASS |

A hex tail: ...3a3432303a73733a3735 00000000 = ":420:ss:75"
B hex: identical to the pinned 7.14.1 fixture

**Test A** proves the device signed exactly the bytes it displayed. Before
`354315c65`, everything after the limit — affiliate name, affiliate fee in bps,
aggregator routing — was hashed and signed with nothing on screen. A 75 bps skim
the user never saw.

**Test B** proves the new `strtok` loop does not mutate what gets hashed: a
legacy memo signs identically to 7.14.1 and gains no spurious screens.

## Card rewritten, as the plan required

The original card subclassed `common.KeepKeyTest`, which demands a DEBUG_LINK
build — not the release artifact, so its evidence would be weaker for a release
gate. That requirement was an artifact of reusing the harness: these memo screens
are entirely host-driven and the pinned serialized tx depends only on the seed.
Rewritten as a plain `KeepKeyClient` script, holds taken physically, no
`set_buttonwait`. Prev tx served from `tests/txcache/` — no network.

## Product-layer note (the reason this test matters twice)

T4 verifies the *firmware* discloses trailing memo fields. It does NOT verify any
product path ever sends them. That distinction is not hypothetical: the XRP
THORChain memo bug was firmware-innocent and lost host-side in hdwallet
(`keepkey-vault#422`), and a firmware-only test would have shown a clean pass
forever.

Vault's `/utxo/sign-transaction` schema types `outputs` as `z.array(z.any())`
(`schemas.ts:124`), so an `opReturnData` field passes through unstripped — the
structural flaw that bit XRP (`.strip()` eating tx fields) is absent here. That
is a schema reading, NOT a hardware measurement; a product-path probe is tracked
separately.

**Plan §3 should gain a sibling section: "verified in firmware but not proven
reachable in the product."** T4 belongs in it today.

## Device left as

Unchanged — mnemonic12, no PIN, no passphrase.
Loading
Loading