Fix XNNPACK FlatBuffer verification and header bounds checking

[lucylq]

1. Add flatbuffer verification to xnnpack graph
2. 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]

🔗 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, 1 Cancelled Job, 3 Pending, 1 Unrelated Failure

As of commit 23e507b with merge base 21d9c64 ():

NEW FAILURE - The following job has failed:

• pull / test-multimodal-linux (gemma3-4b) / linux-job (gh)
  RuntimeError: Command docker exec -t fb833dd18d77400a2c9271ef726f07e59cade897c2032477196cff8b48f7ab10 /exec failed with exit code 139

CANCELLED JOB - The following job was cancelled. Please retry:

• pull / unittest-editable / windows / windows-job (gh)

BROKEN TRUNK - The following job 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.

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

[github-actions]

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]

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.

[GregoryComer]

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

[lucylq]

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

[Copilot]

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]

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.

[GregoryComer]

Valid nit.

[lucylq]

thanks, updated!

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

[lucylq]

@claude review

[claude]

Claude finished @lucylq's task in 3m 50s —— View job

---

Review: Fix XNNPACK FlatBuffer verification and header bounds checking

• Read changed files and gather context
• Analyze header bounds checking logic (XNNHeader.cpp)
• Analyze FlatBuffer verification logic (XNNCompiler.cpp)
• Post review feedback

---

Overall this is a good security hardening PR. The bounds-checking math is correct, the FlatBuffer verifier is properly placed before any traversal, and passing nullptr for the identifier in VerifyBuffer is the right call since both XN00 and XN01 need to be accepted (and the identifier is already validated above). A few issues to address:

Issue 1: Missing minimum flatbuffer_size check — potential OOB read

XNNHeader.cpp — The validation accepts flatbuffer_size == 0 (or any small value). When the header is present (header.ok() path), flatbuffer_size flows into XNNCompiler.cpp where GetBufferIdentifier(flatbuffer_data) is called at line 1839 before the verifier runs. GetBufferIdentifier reads at offset sizeof(uoffset_t) (4 bytes) plus 4 identifier bytes = 8 bytes minimum. A flatbuffer_size of 0–7 would allow an OOB read here.

Copilot flagged both the zero-size issue (in XNNHeader.cpp) and the pre-verification read (in XNNCompiler.cpp). The simplest fix is to enforce a minimum flatbuffer size in XNNHeader::Parse. FlatBuffers requires at least sizeof(uoffset_t) + kFileIdentifierLength = 8 bytes for even the most trivial valid buffer:

// XNNHeader.cpp, after validating flatbuffer region bounds:
constexpr size_t kMinFlatbufferSize = sizeof(uint32_t) + 4; // root offset + identifier
ET_CHECK_OR_RETURN_ERROR(
    flatbuffer_size >= kMinFlatbufferSize,
    InvalidArgument,
    "flatbuffer_size %" PRIu32 " is too small (minimum %zu)",
    flatbuffer_size,
    kMinFlatbufferSize);

Alternatively, add this check in XNNCompiler.cpp before the identifier read at line 1838, which would also protect the NotFound path (though that path is already safe since num_bytes >= kMinSize = 30).

Fix this →

Issue 2: Overlap check is one-directional

XNNHeader.cpp:90-98 — The overlap check assumes flatbuffer always precedes constant data (constant_data_offset >= flatbuffer_offset). If the serializer ever places them in reverse order, this would reject valid data. This seems like an intentional hardening decision (matching the serializer's layout), but worth noting that it's enforcing layout order, not just non-overlap.

If the intent is strictly to prevent overlap (without enforcing order), the check would be:

// No overlap: either constant_data ends before flatbuffer starts,
// or flatbuffer ends before constant_data starts.
(constant_data_offset + constant_data_size <= flatbuffer_offset) ||
(flatbuffer_offset + flatbuffer_size <= constant_data_offset)

If enforcing order is intentional (which is reasonable), consider updating the comment from "does not overlap" to "comes after flatbuffer and does not overlap" to make the intent clear.

Minor nits

• XNNHeader.cpp:82: constant_data_size is uint64_t but constant_data_offset is uint32_t. The format specifier PRIu32 is correct for constant_data_offset, and PRIu64 is correct for constant_data_size — these look good.

• The size parameter is size_t, formatted with %zu — correct.

What looks good

• The bounds-checking arithmetic in XNNHeader::Parse correctly avoids integer overflow by using subtraction instead of addition (flatbuffer_size <= size - flatbuffer_offset instead of flatbuffer_offset + flatbuffer_size <= size).
• Using verifier.VerifyBuffer<fb_xnnpack::XNNGraph>(nullptr) rather than VerifyXNNGraphBuffer correctly handles the dual-identifier (XN00/XN01) situation.
• Tracking flatbuffer_size through both code paths (header present vs. not found) is done correctly.

---