Skip to content

fbpt: reject undersized record lengths before subtraction#3643

Open
alhudz wants to merge 2 commits into
u-root:mainfrom
alhudz:fbpt-length-underflow
Open

fbpt: reject undersized record lengths before subtraction#3643
alhudz wants to merge 2 commits into
u-root:mainfrom
alhudz:fbpt-length-underflow

Conversation

@alhudz

@alhudz alhudz commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Repro: feed an FBPT whose dynamic-string record declares a Length below 34 (the 4-byte record header plus the 30 fixed bytes); see pkg/acpi/fpdt/fbpt/fbpt_test.go.
Expected: the parser rejects the short record.
Actual: recordLength-34 underflows the uint8, so make([]byte, recordLength-34) sizes up to 255 bytes and io.ReadFull copies that much adjacent memory into Description. The same unsigned-underflow shape sits in FindAllFBPTRecords: tablelength-8 (uint32) in the loop bound and HeaderInfo.Length-4 (uint8) before the skip Seek, both fed straight from the firmware table.
Fix: check each length is at least the fixed size it is about to have subtracted, returning an error otherwise.

Verified the recordLength<34 case reads 252 bytes of trailing data into Description before the patch and errors after.

Signed-off-by: alhudz <al.hudz.k@gmail.com>
@codecov

codecov Bot commented Jun 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 33.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.89%. Comparing base (8804437) to head (c499e74).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3643      +/-   ##
==========================================
+ Coverage   60.86%   60.89%   +0.02%     
==========================================
  Files         659      659              
  Lines       46494    46500       +6     
==========================================
+ Hits        28299    28314      +15     
+ Misses      18195    18186       -9     
Flag Coverage Δ
.-amd64 90.90% <ø> (ø)
cmds/...-amd64 52.44% <ø> (-0.02%) ⬇️
integration/generic-tests/...-amd64 30.30% <ø> (ø)
integration/generic-tests/...-arm 33.35% <ø> (ø)
integration/generic-tests/...-arm64 29.42% <ø> (+0.06%) ⬆️
integration/gotests/...-amd64 60.55% <33.33%> (+0.05%) ⬆️
integration/gotests/...-arm 61.17% <33.33%> (+0.04%) ⬆️
integration/gotests/...-arm64 61.29% <33.33%> (+0.05%) ⬆️
pkg/...-amd64 58.77% <33.33%> (+0.05%) ⬆️

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

Components Coverage Δ
everything 65.66% <33.33%> (+0.02%) ⬆️
cmds/exp 34.18% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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