WYSIWYS break on the pixel route: the ERC-7730 adapter drops field values that collide with the navigation vocabulary

Found by GPT-6 Astra in the 2026-09-24 ui-px merge review (FINDING 5), where it was flagged as not independently verified. It is now confirmed end-to-end with the shipped descriptor corpus.

Record: docs/security/adversarial-review/findings/ui-px-merge-2026-09-24.md.

The defect

secure/src/tx/display/erc7730_screens.rs classifies a page row as navigation by matching its text:

pub(crate) fn is_nav_row(row: &[u8]) -> bool {
    row.starts_with(b"> ")
        || row.ends_with(b"> next")
        || row.ends_with(b"> sign")
        || matches!(row, b"L=Cancel" | b"R=Confirm" | b"Long-press:"
                       | b"Long-press to" | b"L cancel/R sign"
                       | b"Long-right" | b"to confirm")
}

The field loop at :333 drops every match. The proven page keeps the row, and the signature commits to it. So a rendered value that collides with that vocabulary is a row the user signs but never sees.

The vocabulary is not reserved — nothing stops a decoded operand from landing in it.

Reachable today, with the shipped corpus

calldata-celo_accounts.json (Celo Accounts, chain 42220, 0x7d21685C17607338b313a7174bAb6620baD0aaB7) admits:

"setName(string name)": { "intent": "Set Account Name",
  "fields": [ { "path": "name", "label": "Name", "format": "raw", "visible": "always" } ] }

format: "raw" puts the attacker-chosen string straight onto a row. setName("> Alice") renders:

NAME

7 bytes

The name is gone.

setName("> Alice") and setName("> Carol") differ on the trusted display in exactly one place: the ERC-8213 32-byte calldata digest. Measured, by diffing the two transcripts:

-0xae284c3e3e2d0a4a1f5032b0ca416b78e50f09f2b5cc00bd08981918faef54a6
+0x88a23209505ecf8de0964aa0db846733e6afdc189a289cfa3ec8513bfc5707b3

Everything else is byte-identical. So the only thing separating the two on the device is a hash the user would have to recompute off-device — blind signing with extra steps, inside the flow whose whole purpose is that it is not.

Wider than the chevron

All four of these field values are signed but never displayed (measured):

"vote > next", "ok > sign", "R=Confirm", "to confirm"

Why nothing caught it

The render-faithfulness differential checks every hex run >= 8 and every decimal run >= 2 of the legacy pages against the screen text. "> Alice" is neither, so alphabetic content is unchecked.

Second consequence of the same textual test: is_confirm_page is "every row empty or nav", so a page whose only content row is "> Alice" classifies as a confirm page.

Tests (on `merge/ui-px-evt`, `secure/src/display_under_test/structured_screens_render_pure_tests.rs`)

Passing, and they stay passing:

control_an_ordinary_field_value_reaches_the_pixel_transcript — the hand-built page shape lifts, so the probes below isolate the value's text
control_celo_set_name_reaches_the_pixel_transcript — setName("Alice") renders end-to-end through the real descriptor, decoder and renderer
measured_two_chevron_names_differ_only_by_the_erc8213_digest — characterisation: asserts the broken behaviour, so the finding is pinned before the fix. Invert it when this is closed.

#[ignore]d, failing until fixed:

a_data_row_beginning_with_a_chevron_is_dropped_from_the_pixel_transcript (adapter)
the_whole_nav_vocabulary_swallows_a_field_value (the full class)
celo_set_name_with_a_chevron_value_reaches_the_transcript (end-to-end)
two_different_celo_names_render_identically

Toward a fix

Every chevron-prefixed nav row in the tree is written to row index 3 (erc20_known.rs, erc20_unknown.rs, value_transfer.rs, erc8213.rs, forced_blind.rs — all pages.buf[p][3]), so position discriminates better than text. But position alone is not a complete fix: the confirm-page vocabulary spans several rows, and a body page's row 3 can hold data.

Proposed, in order of durability:

Structural. The renderer knows which rows it emitted as navigation; carry that out of the page painter instead of re-deriving it by pattern match downstream.
Fail closed. Until then, a body row that matches the nav vocabulary outside the positions the renderer uses should refuse, not silently drop — matching the codebase's "hard refuse rather than render less" discipline.
Widen the differential from hex/decimal runs to all non-nav text, which catches this whole class rather than this instance.

Caveat

The harness renders the leaf with the ! DEV BUILD / Unattested descriptor banner, because synth_bundle synthesises the attestation. The field-rendering path is the same one an attested descriptor takes — the control test proves an ordinary value renders through it — but reachability on a production-attested corpus should be confirmed before severity is finalised.

docs(review): FINDING 5 confirmed end-to-end; wire both records to th…

Severity corrected: critical -> high. I over-rated this when filing it, on the day I confirmed it.

What is genuinely established: the adapter decides what to display by pattern-matching the text it is displaying, and a field value that collides with the navigation vocabulary is silently dropped. That is a real unsoundness in the trusted display and the fix stands.

What I let the framing imply and should not have: that this is an exploitable path to loss today. It is not. The one reachable instance is setName(string) on Celo Accounts — the user still signs exactly the call the descriptor describes; they just cannot read one field. Setting your own Celo account name to an unreadable string is close to harmless on its own.

The reason to fix it is the class, not this instance. The vocabulary is not reserved, so every descriptor admitted from here on is another chance for a value to land in it — and ERC-7730 coverage is expanding deliberately. addStorageRoot(bytes url) and setMetadataURL(string) on this same contract are already format: "raw". A silent drop is the worst possible failure mode for a trusted display, whatever the field happens to be.

No evidence changed; only my rating of it.

test(ui-px): #751 — the pixel adapter drops field values it mistakes …

fix(ui-px): #751 — a field value is never mistaken for the renderer's…

merge: ui-px pixel trusted UI into feat/pq1-board-target

test(ui-px): re-bless three screen goldens that were never green

fix: three findings from the GPT-6 Astra review of today's work

Surviving WYSIWYS collision in the same class — whitespace, not vocabulary

GPT-6 Astra, reviewing the fix in 1f2a57e1, found the adapter still maps two different signed strings to the same displayed field. Verified in source.

erc7730_screens.rs:73:

fn trimmed(row: &[u8; DISPLAY_COLS]) -> &[u8] {
    let mut n = DISPLAY_COLS;
    while n > 0 && row[n - 1] == b' ' { n -= 1; }   // strips TRAILING
    let mut s = 0;
    while s < n && row[s] == b' ' { s += 1; }       // strips LEADING
    &row[s..n]
}

Concrete case, on the same admitted Celo leaf my fix was tested against:

signed	displayed
`setName(" Alice")`	`NAME` / `Alice` / `6 bytes`
`setName("Alice ")`	`NAME` / `Alice` / `6 bytes`

Both are six bytes, the descriptor admits a raw string (calldata-celo_accounts.json:303), and the renderer preserves the bytes and appends the length (formatters.rs:2618). The adapter then trims both ends at :355. The ERC-8213 calldata digest differs, but that is the opaque fallback — the clear-signed field is identical, which is the property this issue is about.

My fix addressed the vocabulary collision and left the normalisation one. The residual I recorded ("a value rendering to exactly > next at row 3") is also narrower than the truth: whitespace-padded variants normalise to > next before the row-3 exemption is applied.

Astra's mitigating finding, which I am recording because it bounds the severity: it did not establish an attacker-controlled admitted string reaching row 3. The dynamic-string painter uses rows 1–2 and puts the length on row 3; enum labels can occupy all three rows but their text is descriptor-controlled (formatters.rs:3082).

It also notes that re-rendering twice cannot catch this: a deterministic omission reproduces identically both times, and the binding check at px_lift.rs:204/:262 establishes the same page buffer and range, not a lossless projection into pixels.

Why this is not a quick fix

Same root cause as the original: the adapter reconstructs meaning from rendered text. Preserving leading whitespace naively would change every existing transcript, and the trim exists because the legacy painter pads to 16 columns. The sound fix is the structural one already named in this issue — the renderer marks which rows it emitted as navigation and which are values, so the adapter never re-derives either from the bytes it is displaying. Escaping meaningful whitespace visibly is the smaller interim step.

Not attempted here. Re-opening this as the remaining work rather than closing the issue on the vocabulary half.

fix(ui-px): #751 second class — whitespace normalisation merged two o…

Whitespace class fixed — `126d76aa`

The two jobs trimmed() was doing at once are now separated:

fn value_text(row: &[u8; DISPLAY_COLS]) -> &[u8] {
    let mut n = DISPLAY_COLS;
    while n > 0 && row[n - 1] == b' ' { n -= 1; }   // trailing grid padding only
    &row[..n]                                       // LEADING whitespace kept
}

and in the field loop:

let probe = trimmed(row);   // classify chrome on the normalised form
let t = value_text(row);    // display the faithful one

So a space-padded " > next" still classifies as chrome and refuses — the vocabulary fix is not weakened — while " Alice" now reaches the screen as " Alice".

No golden moved. That is the useful signal: nothing in the shipped corpus carries leading whitespace in a value row, so this is a no-op for every existing flow and a change only for the colliding case.

New test two_celo_names_differing_only_in_whitespace_render_differently drives Astra's exact pair through the real descriptor, decoder and renderer, and asserts both that the transcripts differ and that the leading space survives into the rendered line.

Verified: host suite 2715 pass / 0 fail, stm32u585 image builds, ui-px frame goldens 7/7 unchanged.

Two limits, stated rather than implied

"Alice " vs "Alice" is still unrecoverable here. The legacy painter left-aligns into a 16-column row and pads with spaces, so those produce byte-identical rows. What separates them is the byte-length row the renderer already emits (6 bytes vs 5 bytes). Astra's pair is fixed because both are six bytes and differ in leading position.
A leading space is a one-column shift. The transcripts now differ in the line bytes that drive the renderer rather than in a digest — but "differs in the transcript" is a stronger claim than "a human will notice". Making it unmistakable needs visible escaping.

What keeps this issue open

Both classes found so far — vocabulary and normalisation — are the same root cause: the adapter reconstructs meaning from rendered text. Each fix has been a patch on one way that inference fails, and Astra's review found the second within hours of the first. The structural fix is still the real answer: the renderer knows which rows it emitted as navigation and which are values, and should say so rather than leave the adapter to infer it from the bytes it is displaying.

fix(erc7730): an intent of exactly 16 chars glued the owner into the …

feat(erc7730): the renderer declares its chrome; the display stops gu…

Regression from the structural fix, found and fixed — `a60121f3`

The chrome mask landed in 4d61878a and broke batch ERC-7730 signing. Recording it here because the mechanism is a direct consequence of the fix's design, and because my stated failure-direction argument was wrong.

What broke

wrap_pages_with_batch_banner copies a batch member's pages one slot down, behind the BATCH SIGN | Tx i of N anchor. It copied buf row by row and knew nothing about the new nav mask, so the wrapped copy arrived with an all-zero mask. erc7730_screens::is_confirm_page_at reads that mask, stopped recognising the body's confirm page, emit returned Err, px_lift reported "px body", and the handler refused the sign.

Caught by make e2e-px, scenario 5e-7730 (batch ERC-7730 trailer matches + signs): NscStatus 0xFFFFFFFF instead of 0, screen id=NOTICE lab="SIGN REFUSED".

The argument I got wrong

4d61878a's message claimed the failure direction was safe:

an unmarked row reads as CONTENT, so a missed write site shows chrome on screen — visible and harmless

That holds for a field row. It does not hold for the confirm page: is_confirm_page_at requires the bits to be set, so a lost mask is a refusal to sign, not a cosmetic artifact. The mask is transcript state, not metadata about it, and every copy of the page bytes has to carry it.

Fix

Three changes in the copy path (secure/src/tx/display/batch.rs):

clear out.nav[..new_len] with the page bytes, so no stale bit from a previous member survives into this one;
shift inner.nav[i] → out.nav[i + 1] alongside the rows;
extend batch_banner_copy_proof to the mask, including nav[0] == 0 for the banner, so a skipped or partial mask copy fails the same FI proof as a dropped row.

Why no host test caught it

Every existing lift test hands Body::Erc7730 an unwrapped transcript. New test the_chrome_mask_survives_the_batch_banner_copy wraps one and asserts the mask shifts with the bytes and the confirm page is still recognised. Negative control: deleting the mask-copy line alone turns it red (page 1 lost its chrome mask, 8 vs 0).

Evidence

	before fix (HEAD)	control `c7f56998`	after fix
`make e2e-px`	panic, 28/47 transcripts	47/47, exit 0	47/47, exit 0

Plus: secure 2717 pass, pqsigner-erc7730 368, pqsigner-ui-px 95 + 7 goldens, transcripts re-exported byte-identical, ui-px stm32u585 image builds.

Correction to an earlier claim on this issue's work

I reported a bench run on 002F0023 as showing the display changes were "byte-identical on hardware". That run used make test-key-speed, which builds ui-semihosting without ui-px, and px_route_confirm is #[cfg(feature = "ui-px")]. No pixel route executed, so the 553/553 frames were legacy 16×4 frames and said nothing about this adapter. The #759 progress-monotonicity result from that run stands (it is on the feature-independent crypto path).

fix(erc7730): the renderer declares how far the intent runs — 95d7831 …

Fixed — all three collision classes, structurally

The root cause this issue identified was correct: the adapter decided what to display by pattern-matching the text it was displaying. That guess produced three distinct WYSIWYS defects, each fixed in turn.

class	fix
a value dropped for matching the nav vocabulary (`"> Alice"`)	`1f2a57e1`
two operands merged by whitespace normalisation (`" Alice"` / `"Alice "`)	`126d76aa`
a 16-character intent gluing the owner line into the caption	`95d7831a` -> `48f6f970`

The fix is option 1 from this issue, not option 2

The renderer now declares which rows it emitted as chrome (Pages::nav, a per-page bitmask), and the signing path reads that declaration. is_nav_row no longer exists on any signing path — it is #[cfg(test)], retained only as the other half of a corpus differential (assert_nav_mask_matches_chrome) that proves the mask agrees with the old inference on every page the renderer actually produces.

That matters for the failure mode. The interim fix (option 2, fail closed) turned a display bug into a signing denial — a legitimate Celo name starting with "> " would have refused. A trusted display owes the user the bytes, not a refusal; the structural fix renders them verbatim.

Tests — the characterisation test was inverted, as this issue asked

Now active and green (no #[ignore] remains):

celo_set_name_with_a_colliding_value_renders_the_name           ok
two_celo_names_differing_only_in_whitespace_render_differently  ok
a_value_that_looks_like_chrome_now_renders_faithfully           ok
control_celo_set_name_lifts                                     ok

$ cargo test -p sphincs-tz-secure --release --no-default-features \
    --features mock-se,debug-log,ui-semihosting -- \
    display_under_test::structured_screens_render_pure_tests \
    display_under_test::erc7730_render_pure_tests
test result: ok. 159 passed; 0 failed; 1 ignored

(The feature set is deliberate: these goldens are authored under it, and running them with default features gives false reds — see the note on ui-px-check in the Makefile. The one ignored test is an unrelated seed-corpus layout diagnostic.)

Two things the fix broke, recorded because they came from its design

4d61878a refused batch ERC-7730 signing. wrap_pages_with_batch_banner copied buf and not nav, so every chrome bit was lost on the batch route. My commit message argued the failure direction was "visible and harmless" — wrong: is_confirm_page_at requires the bits SET, so a lost mask is a refusal to sign. Fixed in a60121f3, with the copy proof extended to cover nav.
95d7831a deleted the owner line from 16-character-intent pages — it traded a caption bug for anti-spoof information loss. Fixed in 48f6f970 by having the renderer declare the intent's extent in two bit-planes instead of inferring it from row fullness.

Still open, deliberately

#762 — the chrome mask now decides what is displayed, and no transcript proof covers it. The mask is load-bearing for WYSIWYS; it should be bound the way the body pages are. That is the residual of this fix and is where the remaining risk in this area lives.

Severity note: I filed this as critical and corrected it to high on 2026-09-28. That correction stands — the reachable instance (setName on Celo Accounts) was never a path to loss. The reason to fix it was the class.

fix(kani): the ui-px harnesses have not compiled since 4d61878; clear…

Navigation Menu

Repository navigation

WYSIWYS break on the pixel route: the ERC-7730 adapter drops field values that collide with the navigation vocabulary #751

The defect

Reachable today, with the shipped corpus

Wider than the chevron

Why nothing caught it

Tests (on `merge/ui-px-evt`, `secure/src/display_under_test/structured_screens_render_pure_tests.rs`)

Toward a fix

Caveat

Surviving WYSIWYS collision in the same class — whitespace, not vocabulary

Why this is not a quick fix

Whitespace class fixed — `126d76aa`

Two limits, stated rather than implied

What keeps this issue open

Regression from the structural fix, found and fixed — `a60121f3`

What broke

The argument I got wrong

Fix

Why no host test caught it

Evidence

Correction to an earlier claim on this issue's work

Fixed — all three collision classes, structurally

The fix is option 1 from this issue, not option 2

Tests — the characterisation test was inverted, as this issue asked

Two things the fix broke, recorded because they came from its design

Still open, deliberately

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

WYSIWYS break on the pixel route: the ERC-7730 adapter drops field values that collide with the navigation vocabulary #751

Description

The defect

Reachable today, with the shipped corpus

Wider than the chevron

Why nothing caught it

Tests (on merge/ui-px-evt, secure/src/display_under_test/structured_screens_render_pure_tests.rs)

Toward a fix

Caveat

Activity

Nicola-Ceornea commented on Sep 28, 2026

Nicola-Ceornea commented on Sep 28, 2026

Surviving WYSIWYS collision in the same class — whitespace, not vocabulary

Why this is not a quick fix

Nicola-Ceornea commented on Sep 28, 2026

Whitespace class fixed — 126d76aa

Two limits, stated rather than implied

What keeps this issue open

Nicola-Ceornea commented on Sep 28, 2026

Regression from the structural fix, found and fixed — a60121f3

What broke

The argument I got wrong

Fix

Why no host test caught it

Evidence

Correction to an earlier claim on this issue's work

Nicola-Ceornea commented on Sep 30, 2026

Fixed — all three collision classes, structurally

The fix is option 1 from this issue, not option 2

Tests — the characterisation test was inverted, as this issue asked

Two things the fix broke, recorded because they came from its design

Still open, deliberately

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Tests (on `merge/ui-px-evt`, `secure/src/display_under_test/structured_screens_render_pure_tests.rs`)

Whitespace class fixed — `126d76aa`

Regression from the structural fix, found and fixed — `a60121f3`