Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security review #15

Merged
merged 3 commits into from
Jan 19, 2021
Merged

Security review #15

merged 3 commits into from
Jan 19, 2021

Conversation

jibeee
Copy link
Contributor

@jibeee jibeee commented Jan 14, 2021

There was a memory corruption in the parse_memo function. The data field of the memo_t structure was not big enough to store a hex hash. Theses hashes are preceeded by "0x", hence two bytes were lacking in the data field.

The PR also fixes a test that was broken, and removes two magic values to make the code a bit more readable.

test_tx_set_all_options requires 16 fields to be displayed, not 15
@TamtamHero TamtamHero merged commit 2dfd10c into LedgerHQ:master Jan 19, 2021
@jibeee jibeee deleted the security-review branch January 19, 2021 11:12
tdejoigny-ledger pushed a commit that referenced this pull request Jul 10, 2024
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