Skip to content

B2CA-2795: Security review - #171

Merged
cedelavergne-ledger merged 6 commits into
developfrom
cev/security_review
Aug 11, 2026
Merged

B2CA-2795: Security review#171
cedelavergne-ledger merged 6 commits into
developfrom
cev/security_review

Conversation

@cedelavergne-ledger

Copy link
Copy Markdown
Contributor

Description

  • Prevent RSA private-exponent disclosure via key-size downgrade
  • Prevent async PIN-confirm callback from authorizing wrong PIN ID
  • CodeQL: remove sizeof-with-side-effects warnings

Checklist

  • App update process has been followed
  • Target branch is develop
  • Application version has been bumped

@codecov-commenter

codecov-commenter commented Aug 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 40.19%. Comparing base (ffb6b62) to head (68a5317).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #171   +/-   ##
========================================
  Coverage    40.19%   40.19%           
========================================
  Files            1        1           
  Lines          209      209           
  Branches        49       49           
========================================
  Hits            84       84           
  Misses         118      118           
  Partials         7        7           
Flag Coverage Δ
unittests 40.19% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…owngrade

READ_ASYM_KEY now requires the stored key size to exactly match the attribute-declared size.
This closes the path where a PW3-authorized attribute downgrade (e.g. RSA-4096 → RSA-2048)
let an unauthenticated READ_ASYM_KEY reinterpret private-exponent bytes as a public modulus.

PUT DATA for algorithm attributes (0xC1/0xC2/0xC3) now atomically erases the owning gpg_key_t
before writing the new attributes, matching the behaviour already implemented in the UX template path.
This ensures stale key material can never coexist with mismatched attributes.
…ong PIN ID

pin_confirm_cb was calling gpg_pin_set_verified with the live io_p2,
which could be overwritten by a new APDU arriving while the confirmation
screen was displayed. A host could send a VERIFY for PW3 (Admin) just
before the user tapped 'Yes' on a User-82 confirmation, resulting in
Admin being marked verified without entering the admin PIN.

The fix has two layers:
1. Snapshot: io_p2 is captured in ux_pinconfirm_p2 at the moment the UI is drawn.
   All callbacks use this snapshot instead of the live io_p2.
2. Guard: gpg_dispatch rejects any APDU received while ui_pending is set
   with SWO_CONDITIONS_NOT_SATISFIED, so no command can complete behind a pending prompt.
   ui_pending is set at draw time and cleared at the start of each UI callback.
Replace sizeof(field_expression) checks with named constants so sizeof
is never applied to expressions containing pointer dereferences or variable reads.
Update gpg_types.h struct definition to use the same constants.
Fixes cross-user cache disclosure and parse-time crash in undici's
cache interceptor (transitive dep via jsdom and vitest).
@github-actions

Copy link
Copy Markdown
Contributor

Code coverage report

Code Coverage

Per-file coverage
Package Line Rate Branch Rate Health
src 25% 9%
Summary 25% (56 / 227) 9% (9 / 101)

@cedelavergne-ledger
cedelavergne-ledger merged commit aab2c1f into develop Aug 11, 2026
84 checks passed
@cedelavergne-ledger
cedelavergne-ledger deleted the cev/security_review branch August 11, 2026 15:29
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.

3 participants