Skip to content

Fix uint64_t overflow in SectionBMC bounds check - #9962

Closed
stsoe wants to merge 1 commit into
Xilinx:masterfrom
stsoe:codeql-xclbinutil-2
Closed

Fix uint64_t overflow in SectionBMC bounds check#9962
stsoe wants to merge 1 commit into
Xilinx:masterfrom
stsoe:codeql-xclbinutil-2

Conversation

@stsoe

@stsoe stsoe commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Problem solved by the commit

copyBufferUpdateMetadata() computed pHdr->m_offset + pHdr->m_size as a uint64_t addition before comparing against _origSectionSize. If both fields are near UINT64_MAX the addition wraps silently, causing the bounds check to pass on a malformed section.

Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered Found during triage of AIESW-41122/41123 (false positives). The overflow was latent in the original check predating bb1791f.

How problem was solved, alternative solutions (if any) and why they were rejected Replaced the addition with an overflow-safe two-part check:

m_size > _origSectionSize || m_offset > _origSectionSize - m_size

Risks (if any) associated the changes in the commit Low. Semantically equivalent for all non-overflow inputs; only rejects crafted inputs that previously slipped past the check.

What has been tested and how, request additional testing if necessary Built xclbinutil successfully.

Documentation impact (if any)

None

#### Problem solved by the commit
copyBufferUpdateMetadata() computed `pHdr->m_offset + pHdr->m_size`
as a uint64_t addition before comparing against _origSectionSize.
If both fields are near UINT64_MAX the addition wraps silently,
causing the bounds check to pass on a malformed section.

#### Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered
Found during triage of AIESW-41122/41123 (false positives). The
overflow was latent in the original check predating bb1791f.

#### How problem was solved, alternative solutions (if any) and why they were rejected
Replaced the addition with an overflow-safe two-part check:
  m_size > _origSectionSize || m_offset > _origSectionSize - m_size

#### Risks (if any) associated the changes in the commit
Low. Semantically equivalent for all non-overflow inputs; only
rejects crafted inputs that previously slipped past the check.

#### What has been tested and how, request additional testing if necessary
Built xclbinutil successfully.

#### Documentation impact (if any)
None

Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@stsoe
stsoe requested a review from xfreid as a code owner August 5, 2026 21:45
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@stsoe

stsoe commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

Replaced by #9966

@stsoe stsoe closed this Aug 6, 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