test: cover the disclosure behaviour 7.14.2 adds - #216
Open
BitHighlander wants to merge 1 commit into
Open
Conversation
This was referenced Aug 20, 2026
BitHighlander
force-pushed
the
release/7.14.2-tests
branch
from
August 20, 2026 22:18
99906e4 to
d1b5da1
Compare
BitHighlander
added a commit
to keepkey/keepkey-firmware
that referenced
this pull request
Aug 20, 2026
Every defect closed here is the same shape: the device signed bytes it never displayed, or displayed text it could not vouch for. EVM omitted chain_id no longer yields a pre-EIP-155 signature; it is refused before any screen is drawn arbitrary contract data requires AdvancedMode, and the check gates rather than warns 0x clear-signing is bound to complete calldata and to both tokens resolving calldata beyond the initial chunk is no longer described from hardcoded ABI offsets EIP-712 the structured JSON path is withdrawn: it could not guarantee that a displayed value was the value being hashed, and one screen took its title from the attacker-supplied domain name EthereumSignTypedHash requires AdvancedMode and shows an explicit blind-sign screen first Display message signing renders every byte, escaping non-printables, instead of stopping at the first NUL bodies too long for the screen are paged with n/m titles rather than silently clipped the fit measurement is draw_string()'s own walk with pixel writes off, so measuring and drawing cannot disagree Altcoins THORChain and Maya memos keep their last character and refuse a declared length that misdescribes its own content; trailing memo fields are disclosed Osmosis prints the exact signed integer beside the exact denom Cosmos IBC shows the receiver instead of printing the sender twice Solana SPL amounts are scaled by the decimals in the signed instruction, and never render a nonzero transfer as zero Binance denom formatting no longer writes past its buffer Setup and consent an injected RecoveryDevice cannot write settings mid-ceremony a cancelled passphrase confirmation is not cached cancelled authenticator and Uniswap operations do not complete Reproduced on signed v7.14.1 before the fixes and verified on 7.14.2 silicon after them. CI now captures the OLED for these paths; it previously captured none of them, because the harness read the firmware version with grep -oP, which BusyBox lacks, and silently fell back to 7.14.0. Not claimed: independent re-exploitation and a final adversarial round are incomplete, and the fit measurement covers layout_standard_notification only, so custom-layout address and xpub bodies still clip without warning. deps/python-keepkey pins keepkey/python-keepkey#216, which merges once this is green.
The device now shows content it previously signed without displaying. These
tests assert that, and capture the screens as evidence.
New:
test_msg_display_disclosure a payload and the same payload with a hidden
suffix must not produce identical screens --
NUL-terminated, whitespace-padded, over-long,
and newline-padded variants
test_msg_ping a long body is paged with n/m titles; a short one
is not
Updated for behaviour that deliberately changed:
EVM tests now pass chain_id explicitly and no longer assert pre-EIP-155
signatures; policy-gated tests opt in; the EIP-712 tests assert the refusal
that replaced the withdrawn parser; the XRP THORChain memo test is skipped
rather than weakened, with its assertion intact.
Corrected: both add_liquidity fixtures declared a 59-byte ABI memo length for a
58-byte memo, so the length word claimed a padding byte as content. Firmware now
refuses that, so the fixtures were wrong rather than the check. Signatures
repinned from the corrected calldata -- computed twice independently, by the
emulator and by a physical device on signed v7.14.1.
Report: SECTIONS gains the display-binding entries so CI captures these screens,
and --screenshot-audit fails the build when a test declares screens it never
produces. That gap was real: the suites this release changed captured nothing.
BitHighlander
force-pushed
the
release/7.14.2-tests
branch
from
August 20, 2026 22:40
d1b5da1 to
87ff249
Compare
BitHighlander
added a commit
to keepkey/keepkey-firmware
that referenced
this pull request
Aug 20, 2026
Every defect closed here is the same shape: the device signed bytes it never displayed, or displayed text it could not vouch for. EVM omitted chain_id no longer yields a pre-EIP-155 signature; it is refused before any screen is drawn arbitrary contract data requires AdvancedMode, and the check gates rather than warns 0x clear-signing is bound to complete calldata and to both tokens resolving calldata beyond the initial chunk is no longer described from hardcoded ABI offsets EIP-712 the structured JSON path is withdrawn: it could not guarantee that a displayed value was the value being hashed, and one screen took its title from the attacker-supplied domain name EthereumSignTypedHash requires AdvancedMode and shows an explicit blind-sign screen first Display message signing renders every byte, escaping non-printables, instead of stopping at the first NUL bodies too long for the screen are paged with n/m titles rather than silently clipped the fit measurement is draw_string()'s own walk with pixel writes off, so measuring and drawing cannot disagree Altcoins THORChain and Maya memos keep their last character and refuse a declared length that misdescribes its own content; trailing memo fields are disclosed Osmosis prints the exact signed integer beside the exact denom Cosmos IBC shows the receiver instead of printing the sender twice Solana SPL amounts are scaled by the decimals in the signed instruction, and never render a nonzero transfer as zero Binance denom formatting no longer writes past its buffer Setup and consent an injected RecoveryDevice cannot write settings mid-ceremony a cancelled passphrase confirmation is not cached cancelled authenticator and Uniswap operations do not complete Reproduced on signed v7.14.1 before the fixes and verified on 7.14.2 silicon after them. CI now captures the OLED for these paths; it previously captured none of them, because the harness read the firmware version with grep -oP, which BusyBox lacks, and silently fell back to 7.14.0. Not claimed: independent re-exploitation and a final adversarial round are incomplete, and the fit measurement covers layout_standard_notification only, so custom-layout address and xpub bodies still clip without warning. deps/python-keepkey pins keepkey/python-keepkey#216, which merges once this is green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Test changes for the 7.14.2 security release
(keepkey/keepkey-firmware#458). One commit.
New
test_msg_display_disclosure— a payload and the same payload with ahidden suffix must not produce identical screens. Covers NUL-terminated,
whitespace-padded, over-long and newline-padded variants.
test_msg_ping— a long body is paged withn/mtitles; a short one isnot. The negative case matters: a pager that numbered every confirmation would
cost users an extra press on ordinary approvals.
Updated, because the behaviour deliberately changed
EVM tests pass
chain_idexplicitly and no longer assert pre-EIP-155signatures. Policy-gated tests opt in. The EIP-712 tests assert the refusal that
replaced the withdrawn parser. The XRP THORChain memo test is skipped rather
than weakened — its assertion is intact for when the feature lands.
Corrected fixtures
Both
add_liquidityfixtures declared a 59-byte ABI memo length for a 58-bytememo, so the length word claimed a padding byte as content. Firmware now refuses
that encoding, which made these fixtures wrong rather than the check.
Signatures repinned from the corrected calldata, computed twice independently —
by the emulator and by a physical device running signed v7.14.1. Two signers
agreeing is what makes repinning safe rather than circular.
Report
SECTIONSgains the display-binding entries so CI captures these screens, and--screenshot-auditfails the build when a test declares screens it neverproduces.
That gap was real: every suite this release changed captured zero screens,
because the harness read the firmware version with
grep -oP, which BusyBoxlacks, and silently fell back to
7.14.0.