upstream #384: fault injection hardening, nanopb memleak, BIP39 validation#64
Closed
BitHighlander wants to merge 6 commits intodevelopfrom
Closed
upstream #384: fault injection hardening, nanopb memleak, BIP39 validation#64BitHighlander wants to merge 6 commits intodevelopfrom
BitHighlander wants to merge 6 commits intodevelopfrom
Conversation
- Point device-protocol and python-keepkey submodules to upstream master (includes BIP-85, Solana, Tron, TON wire IDs and proto definitions) - Add nanopb .options files for Solana, Tron, TON (field size constraints) - Add Bip85Mnemonic.mnemonic max_size:241 to messages.options - Update lib/transport/CMakeLists.txt with new proto sources, options, headers, and protoc compilation commands - Fix CI: use pre-installed clang-format instead of apt-get install (eliminates 3-minute timeout on GitHub runners) - Update Zcash transparent branch ID from Sapling to NU6
…double-hash (F3) Replace early-return ECDSA verify chain with bitwise-OR accumulation and sentinel counter. A single voltage glitch can no longer skip one branch to fall through to SIG_OK — attacker must now corrupt all three verify results AND the sentinel. Add double SHA-256 computation with constant-time memcmp_s comparison to detect transient faults during hash computation. References: VULN-21020, fault-injection-assessment.md finding F3
…eck (F5) Remove redundant ~1 second full ECDSA re-verification in firmware main. The bootloader already performed authoritative signature verification before jumping here. Replace with fast metadata presence check that validates signature indices are present and distinct. Eliminates the wide timing window (VULN-21020 class) where a voltage glitch during the long-running crypto re-check could bypass protections. References: VULN-21020, fault-injection-assessment.md finding F5
Moves which_field tag assignment after the conditional memset and only resets when the oneof variant changes. Prevents allocated memory from being zeroed before release when PB_ENABLE_MALLOC is active. Ref: keepkey#361 Upstream: nanopb 4fe23595732b6f1254cfc11a9b8d6da900b55b0c
The condition on line 578 was inverted — `!enforce_wordlist` meant the wordlist check was skipped when enforce_wordlist was true (the default). Invalid words were silently accepted and only caught later as a generic "Invalid mnemonic" checksum error, giving users no indication which word was wrong. Flip the condition so that when enforce_wordlist is set, words that fail auto-complete against the BIP39 list are rejected immediately with a clear error before the mnemonic is finalized. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Previously, wordlist validation only happened at finalization after all words were entered. Invalid words were silently accepted and the user had to complete the entire 12/24-word entry before getting a vague "Invalid mnemonic" error with no indication which word was wrong. Add a BIP39 check in the space handler of recovery_character() that rejects immediately with "Word not found in BIP39 wordlist" when the decoded characters don't match any entry. Combined with the enforce_wordlist condition fix, both per-word and finalization validation now work correctly.
Owner
Author
|
Already integrated — all 5 security hardening commits are in our develop (signatures_ok, nanopb leak, BIP39 validation). |
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.
Summary
Cherry-pick of upstream keepkey#384
Security hardening: fault injection countermeasures, nanopb memory leak fix, BIP39 recovery validation.
Test plan