Skip to content

Fix bugs in libhoth_extract_ot_bundle#209

Open
sandumjacob wants to merge 1 commit intogoogle:mainfrom
sandumjacob:extract_ot_bundle_fix
Open

Fix bugs in libhoth_extract_ot_bundle#209
sandumjacob wants to merge 1 commit intogoogle:mainfrom
sandumjacob:extract_ot_bundle_fix

Conversation

@sandumjacob
Copy link
Contributor

  • Fix integer overflow, buffer overread in libhoth_extract_ot_bundle
  • Add regression tests.

@sandumjacob sandumjacob force-pushed the extract_ot_bundle_fix branch 2 times, most recently from 259b59d to cf295be Compare February 19, 2026 19:05
@sandumjacob sandumjacob force-pushed the extract_ot_bundle_fix branch from cf295be to cc50b3c Compare February 19, 2026 19:07
@xorptr xorptr self-requested a review February 24, 2026 21:08

// Checks that the image offset doesn't cause
// the offset calculations to read beyond the image buffer.
if ((offset + OPENTITAN_OFFSET_APP_FW + OPENTITAN_OFFSET_VERSION_MINOR + 4) >
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that this check relies on 2 implicit conditions for correctness (which hold true currently):

  1. (OPENTITAN_OFFSET_HEADER_DATA + 4) <= image_size, so that offset can be read properly. This is currently true since OPENTITAN_OFFSET_HEADER_DATA < OPENTITAN_OFFSET_APP_FW and image_size >= smallest_fw
  2. OPENTITAN_OFFSET_VERSION_MINOR > OPENTITAN_OFFSET_VERSION_MAJOR so that ROM_EXT and APP version can be read properly.

Would it be a good idea to check these conditions explicitly (using static_assert or in some other manner)?

// the offset calculations to read beyond the image buffer.
if ((offset + OPENTITAN_OFFSET_APP_FW + OPENTITAN_OFFSET_VERSION_MINOR + 4) >
image_size) {
fprintf(stderr, "Image offset is invalid");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Would it be a good idea to print offset in the error messages?

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