Skip to content

Harden SBAT handling against truncation and impossible to revoke levels - #829

Open
julian-klode wants to merge 2 commits into
rhboot:mainfrom
julian-klode:sbat-overflow
Open

julian-klode wants to merge 2 commits into
rhboot:mainfrom
julian-klode:sbat-overflow

Conversation

@julian-klode

Copy link
Copy Markdown
Collaborator

Commit 1 removes the truncation to UINTN to UINT16, and uses a custom parser to avoid the discrepancy between atoi() in the test environment and atoi() in EFI.

Commit 2 hard rejects images with SBAT level UINT16_MAX or greater such that UINT16_MAX can be used to revoke the last binary for that component.

@codecov

codecov Bot commented Sep 3, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.42%. Comparing base (72fd4b7) to head (36dcd88).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #829      +/-   ##
==========================================
- Coverage   59.48%   59.42%   -0.07%     
==========================================
  Files          24       24              
  Lines        4226     4283      +57     
  Branches      655      663       +8     
==========================================
+ Hits         2514     2545      +31     
+ Misses       1471     1470       -1     
- Partials      241      268      +27     

☔ 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.

@julian-klode

Copy link
Copy Markdown
Collaborator Author

Not sure the "invalid" sbat variable handling makes sense, should we detect that earlier and re-write the sbat variable if it is corrupted?

@julian-klode

Copy link
Copy Markdown
Collaborator Author

Cleaning up the missing partial line aand some weird typing

verify_single_entry() stored atoi() results in UINT16, truncating
65537 to 1 and wrapping -1 to 65535.  Parse with a shim-local
helper instead that rejects negatives, huge values, and trailing
garbage; strtol() is unimplemented in EFI and atoi() differs
between EFI (AsciiStrDecimalToUintn()) and test (libc) builds.
Failed parses are still treated as zero.

Assisted-By: LLM
Signed-off-by: Julian Andres Klode <julian.klode@canonical.com>
An image using the maximum 16-bit generation could never be
revoked, as revocation requires a higher policy generation.
Reject generations above SBAT_GENERATION_MAX (UINT16_MAX - 1) so
one generation of headroom always remains.  The policy may still
use UINT16_MAX itself to disallow a component entirely.

Assisted-By: LLM
Signed-off-by: Julian Andres Klode <julian.klode@canonical.com>
@julian-klode

Copy link
Copy Markdown
Collaborator Author

All lines are fully covered now. If we want we could also allow 32-bit integers, but this keeps the 16-bit limit.

@vathpela vathpela added this to the shim 17 milestone Oct 2, 2026
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