Skip to content

fix(zingolib): constrain wallet reader & recovery, and add validation method - #2749

Open
dorianvp wants to merge 5 commits into
devfrom
bb/add-wallet-file-validation-thr_dfg7v7d7jv
Open

fix(zingolib): constrain wallet reader & recovery, and add validation method#2749
dorianvp wants to merge 5 commits into
devfrom
bb/add-wallet-file-validation-thr_dfg7v7d7jv

Conversation

@dorianvp

Copy link
Copy Markdown
Member

No description provided.

dorianvp and others added 3 commits August 28, 2026 00:40
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
LightWallet::validate promises an io::Result whose error names the byte
offset reached, but the read path it wraps carried seven expect() calls
that panic on crafted-but-reachable inputs: a pre-32 birthday above
u32::MAX, account ids with the hardened bit set in the unified key
store and in both address maps, a hardened transparent address index, a
version 35 file whose key store vector holds no account 0, and a stored
min_confirmations of zero. Each site now returns an InvalidData error
instead, and the three identical account id reads collapse into one
read_account_id helper.

Five regression tests craft minimal wallet prefixes that reach the five
shallow sites and assert validate returns an error rather than
aborting. The birthday and min_confirmations sites sit behind fields
whose grammars are impractical to hand-craft, so they are fixed by the
same mechanical transformation without dedicated fixtures.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
zancas
zancas previously approved these changes Aug 28, 2026

@zancas zancas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I particularly like the CountingReader pattern.

(re) running tests locally.

fix: return InvalidData instead of panicking on crafted wallet bytes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants