Skip to content

bugfix: bound Delta Mode PIN reconstruction - #748

Open
scgbckbone wants to merge 1 commit into
Coldcard:masterfrom
scgbckbone:bugfix/delta-pin-bounds
Open

scgbckbone wants to merge 1 commit into
Coldcard:masterfrom
scgbckbone:bugfix/delta-pin-bounds

Conversation

@scgbckbone

Copy link
Copy Markdown
Collaborator

Bound bootloader Delta Mode PIN reconstruction and fail closed on malformed PIN data. Removes the unnecessary terminator write and prevents the reverse walk from moving before the buffer.

This is a defense-in-depth correctness fix, not an exploitable security issue: supported PIN entry and trick-PIN provisioning cannot produce the malformed inputs needed to reach these bounds.

Validation: Mk4 and Q pins.o compile. No Python test applies because the simulator does not execute bootloader C.

@scgbckbone scgbckbone left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

OK by KIMI 3

@doc-hex doc-hex added the bootloader Affects bootloader ROM area, set in stone at factory. label Aug 17, 2026

for(int i=0; i<4; i++, p--) {
if(*p == '-') p--;
// A valid Delta Mode PIN must provide four digits.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

untrue

cursor--;

int here = replacement & 0xf;
int replacement_digit = replacement & 0xf;

@doc-hex doc-hex Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I prefer here to replacement_digit but I'm not paid by the token.

@doc-hex

doc-hex commented Sep 1, 2026

Copy link
Copy Markdown
Contributor
  • really not convinced this is at all a useful change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bootloader Affects bootloader ROM area, set in stone at factory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants