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

Fix crash with appInfo() reading past end of buffer #51

Closed
wants to merge 1 commit into from

Conversation

NullSoldier
Copy link

This is a bug because the APDU proxy does not send back any flags unless you use HAVE_BOLOS define when building the SDK for the device. This does not seem to be built in on the NANO S PLUS target. That means using appInfo() will crash because it assumes you get flags back but they are never sent.

See supporting documentation and code here,

This is a bug because the APDU proxy does not send back any flags unless
you use HAVE_BOLOS define when building the SDK for the device. This
does not seem to be built in on the NANO S PLUS target. That means using
appInfo() will crash because it assumes you get flags back but they are
never sent.

See supporting documentation and code here,
- https://github.com/LedgerHQ/ledger-secure-sdk/blob/cb5cfae5750d57e4c03460c5c93098fea7a8abcb/include/sdk_apdu_commands.h#L8-L32
- https://github.com/LedgerHQ/ledger-secure-sdk/blob/cb5cfae5750d57e4c03460c5c93098fea7a8abcb/src/os_io_seproxyhal.c#L1156-L1161
@emmanuelm41
Copy link
Member

@NullSoldier thanks for the contribution. It is not really necessary, as appFlags should be there. They are missed on the Rust bindings. PR adding them in ledger LedgerHQ/ledger-device-rust-sdk#198. Once merge, this should be fix automatically. I think the PR is no longer needed.

@NullSoldier NullSoldier closed this Oct 7, 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