Fix XNNPACK FlatBuffer verification and header bounds checking by lucylq · Pull Request #18784 · pytorch/executorch

lucylq · 2026-04-08T23:47:04Z

Add flatbuffer verification to xnnpack graph
Check the flatbuffer and constant data region are valid (within flatbuffer size, and do not overlap with each other)

This PR was authored with the assistance of Claude.

pytorch-bot · 2026-04-08T23:47:08Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/18784

📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

Rolling out OSDC (ARC) runners on pull workflow for PyTorch trunk commits

❌ 1 New Failure, 2 Unrelated Failures

As of commit 40c83d4 with merge base 21d9c64 ():

NEW FAILURE - The following job has failed:

pull / test-samsung-models-linux / linux-job (gh)
test_w2l_fp16

BROKEN TRUNK - The following jobs failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

pull / unittest / windows / windows-job (gh) (trunk failure)
##[error]The operation was canceled.
pull / unittest-editable / windows / windows-job (gh) (trunk failure)
##[error]The operation was canceled.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

github-actions · 2026-04-08T23:48:10Z

This PR needs a `release notes:` label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Copilot

Pull request overview

This PR hardens XNNPACK delegate deserialization by validating wrapper-header regions and verifying the embedded FlatBuffer before traversing it, aiming to prevent OOB reads on malformed/corrupt inputs.

Changes:

Add bounds/overlap checks for the flatbuffer and constant-data regions in XNNHeader::Parse.
Track the effective flatbuffer byte length and run FlatBuffers verifier before GetXNNGraph access in XNNCompiler::compileModel.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
backends/xnnpack/runtime/XNNHeader.cpp	Adds header-based range checks for flatbuffer/constant-data offsets & sizes, including non-overlap enforcement.
backends/xnnpack/runtime/XNNCompiler.cpp	Introduces FlatBuffer verification using the computed flatbuffer size before parsing the graph.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-08T23:52:19Z

backends/xnnpack/runtime/XNNCompiler.cpp

+  // Verify the FlatBuffer data integrity before accessing it. Without this,
+  // malformed data could cause out-of-bounds reads when traversing the
+  // FlatBuffer's internal offset tables.
+  flatbuffers::Verifier verifier(flatbuffer_data, flatbuffer_size);
+  ET_CHECK_OR_RETURN_ERROR(
+      fb_xnnpack::VerifyXNNGraphBuffer(verifier),
+      DelegateInvalidCompatibility,
+      "FlatBuffer verification failed; data may be truncated or corrupt");


flatbuffers::Verifier is run via fb_xnnpack::VerifyXNNGraphBuffer(verifier), but the generated verifier typically enforces the schema’s file_identifier. In this repo, the runtime schema uses file_identifier "XN00" while the Python serializer produces buffers with identifier XN01 (and this function already intends to support both). As written, verification will likely reject valid XN01 payloads and break backward compatibility.

Consider verifying the buffer without enforcing the identifier (e.g., verifier.VerifyBuffer<fb_xnnpack::XNNGraph>(nullptr)), or selecting the expected identifier string ("XN00"/"XN01") based on the earlier identifier check and passing it to VerifyBuffer<...>(expected_id) instead of calling the generated VerifyXNNGraphBuffer.

Can you double check BC with this change? We don't have good CI coverage for this currently (I need to fix this).

Ah this is updated to use verifier.VerifyBuffer.. which doesn't specify the magic number

Copilot · 2026-04-08T23:52:19Z

backends/xnnpack/runtime/XNNCompiler.cpp

      "XNNPACK Delegate Serialization Format version identifier '%.4s' != expected XN00 or XN01'",
      flatbuffers::GetBufferIdentifier(flatbuffer_data));


flatbuffers::GetBufferIdentifier(flatbuffer_data) is used before any minimum-length/verification check. If flatbuffer_size is < 8 (e.g., truncated/corrupt input), this can read past the provided buffer. Add an explicit minimum-size check before calling it (at least sizeof(flatbuffers::uoffset_t) + flatbuffers::kFileIdentifierLength), or perform verification/bounds checking prior to identifier access.

Copilot · 2026-04-08T23:52:19Z

backends/xnnpack/runtime/XNNHeader.cpp

+  ET_CHECK_OR_RETURN_ERROR(
+      flatbuffer_offset <= size && flatbuffer_size <= size - flatbuffer_offset,
+      InvalidArgument,
+      "flatbuffer_offset: %u and flatbuffer_size: %u are invalid for buffer of size: %zu",
+      flatbuffer_offset,
+      flatbuffer_size,
+      size);


The new error messages print uint32_t fields using %u. In this codebase there are already places using PRIu32 for uint32_t formatting; using the PRIu32 macros here as well avoids type/format mismatches on platforms where uint32_t isn’t unsigned int and keeps formatting consistent.

thanks, updated!

…XECUTORCH-33, TOB-EXECUTORCH-34) TOB-EXECUTORCH-33: XNNCompiler::compileModel() processed FlatBuffer data via fb_xnnpack::GetXNNGraph() without first running the FlatBuffer verifier. A malformed or truncated payload could cause out-of-bounds reads when the FlatBuffer library follows internal offset tables. This adds a flatbuffers::Verifier pass (matching the pattern used in program.cpp) before any FlatBuffer accessors are called, and tracks the flatbuffer_size so the verifier knows the exact bounds of the serialized data. TOB-EXECUTORCH-34: XNNHeader::Parse() read flatbuffer_offset, flatbuffer_size, constant_data_offset, and constant_data_size from untrusted header bytes but never validated that the resulting regions actually fit within the provided buffer. Crafted offset/size values could point past the end of the buffer, leading to out-of-bounds reads in compileModel(). This adds overflow-safe bounds checks that ensure both the flatbuffer and constant data regions fall within [0, size). This PR was authored with the assistance of Claude.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

backends/xnnpack/runtime/XNNHeader.cpp

+  // Validate that flatbuffer region does not overflow or exceed the buffer.
+  ET_CHECK_OR_RETURN_ERROR(
+      flatbuffer_offset <= size && flatbuffer_size <= size - flatbuffer_offset,
+      InvalidArgument,
+      "flatbuffer_offset: %" PRIu32 " and flatbuffer_size: %" PRIu32
+      " are invalid for buffer of size: %zu",
+      flatbuffer_offset,
+      flatbuffer_size,
+      size);


meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 8, 2026

lucylq force-pushed the security33-34 branch from 3c586d6 to dca5ef1 Compare April 8, 2026 23:47

lucylq marked this pull request as ready for review April 8, 2026 23:48

lucylq requested a review from digantdesai as a code owner April 8, 2026 23:48

lucylq requested review from GregoryComer and Copilot April 8, 2026 23:48

Copilot started reviewing on behalf of lucylq April 8, 2026 23:48 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

lucylq force-pushed the security33-34 branch from dca5ef1 to 0119731 Compare April 10, 2026 18:42

Copilot AI review requested due to automatic review settings April 10, 2026 21:54

lucylq force-pushed the security33-34 branch from 0119731 to 40c83d4 Compare April 10, 2026 21:54

Copilot started reviewing on behalf of lucylq April 10, 2026 21:54 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix XNNPACK FlatBuffer verification and header bounds checking#18784

Fix XNNPACK FlatBuffer verification and header bounds checking#18784
lucylq wants to merge 1 commit intomainfrom
security33-34

lucylq commented Apr 8, 2026

Uh oh!

pytorch-bot bot commented Apr 8, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Apr 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

GregoryComer Apr 10, 2026

Uh oh!

lucylq Apr 10, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

GregoryComer Apr 10, 2026

Uh oh!

lucylq Apr 10, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		"XNNPACK Delegate Serialization Format version identifier '%.4s' != expected XN00 or XN01'",
		flatbuffers::GetBufferIdentifier(flatbuffer_data));

Conversation

lucylq commented Apr 8, 2026

Uh oh!

pytorch-bot bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/18784

❗ 1 Active SEVs

❌ 1 New Failure, 2 Unrelated Failures

Uh oh!

github-actions bot commented Apr 8, 2026

This PR needs a release notes: label

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

GregoryComer Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

lucylq Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

GregoryComer Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

lucylq Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pytorch-bot bot commented Apr 8, 2026 •

edited

Loading

This PR needs a `release notes:` label

lucylq Apr 10, 2026 •

edited

Loading