Check flatbuffer offset when verification is minimal by lucylq · Pull Request #17129 · pytorch/executorch

lucylq · 2026-02-03T01:00:12Z

Summary

Unconditionally verify that the offset is within the flatbuffer bounds. Setting InternalConsistency is a binary size burden (20-30kb), so check this separately when we have minimal verification.

Error out if InternalConsistency is requested, but verification is disabled.

Test plan

cd build
cmake .. -DEXECUTORCH_BUILD_TESTS=ON
cmake --build . --target program_test
ctest -R program_test -V

pytorch-bot · 2026-02-03T01:00:16Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR

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

❌ 1 Awaiting Approval, 9 New Failures, 21 Pending

As of commit 13ba046 with merge base 6c02866 ():

AWAITING APPROVAL - The following workflow needs approval before CI can run:

Claude Code (gh)

NEW FAILURES - The following jobs have failed:

Lint / lintrunner (gh)
>>> Lint for runtime/executor/program.cpp:
pull / test-binary-size-linux-gcc / linux-job (gh)
/pytorch/executorch/kernels/portable/cpu/op_convolution.cpp:175:41: error: comparison of integer expressions of different signedness: ‘ssize_t’ {aka ‘long int’} and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
pull / test-coreml-bc-macos (macos-m2-stable) / macos-job (gh)
RuntimeError: Command bash /Users/ec2-user/runner/_work/_temp/exec_script failed with exit code 1
pull / test-moshi-linux / linux-job (gh)
test_exported_decoder_xnnpack
pull / test-multimodal-linux (gemma3-4b) / linux-job (gh)
RuntimeError: Command docker exec -t cdbb7a819239a13d0fac331cc6fe7b369b31ad5688e6304edc77c984a5e6d501 /exec failed with exit code 139
pull / unittest / linux / linux-job (gh)
runtime/test/test_runtime.py::RuntimeTest::test_load_program_with_path
pull / unittest / windows / windows-job (gh)
[ FAILED ] KernelTempMemoryAllocatorIntegrationTest.UsingTempMemoryAllocator
pull / unittest-editable / linux / linux-job (gh)
runtime/test/test_runtime.py::RuntimeTest::test_load_program_with_path
pull / unittest-editable / windows / windows-job (gh)
[ FAILED ] KernelTempMemoryAllocatorIntegrationTest.UsingTempMemoryAllocator

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

github-actions · 2026-02-03T01:00:53Z

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 pull request adds minimal verification for flatbuffer root offsets to prevent crashes when InternalConsistency verification is disabled (to save binary size). The PR also changes the behavior when InternalConsistency verification is requested but not available - it now returns an error instead of just logging a warning.

Changes:

Added bounds checking for the flatbuffer root offset in minimal verification mode to catch corruption early
Changed behavior when InternalConsistency verification is unavailable from logging at Info level to logging at Error level and returning Error::NotSupported
Added a test to verify that invalid root offsets are caught by minimal verification

Reviewed changes

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

File	Description
runtime/executor/program.cpp	Implements minimal verification by checking root offset bounds and makes InternalConsistency unavailability an error instead of a warning
runtime/executor/test/program_test.cpp	Adds test case to verify that obviously invalid root offsets are rejected during minimal verification

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

runtime/executor/program.cpp

Copilot

Pull request overview

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

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

Copilot · 2026-02-06T23:56:52Z

runtime/executor/program.cpp

+    // Minimal verification: check that the root table offset is within bounds.
+    // The first 4 bytes of the flatbuffer contain an offset to the root table.
+    ET_CHECK_OR_RETURN_ERROR(
+        program_data->size() >= sizeof(flatbuffers::uoffset_t),
+        InvalidProgram,
+        "Program data size %zu is too small for flatbuffer header",
+        program_data->size());
+    uint32_t root_offset = flatbuffers::ReadScalar<flatbuffers::uoffset_t>(
+        program_data->data());
+    // The root table is at buf + root_offset, and must have at least a
+    // vtable offset (soffset_t) at that position.
+    ET_CHECK_OR_RETURN_ERROR(
+        root_offset <= program_data->size() - sizeof(flatbuffers::uoffset_t),
+        InvalidProgram,
+        "Root table offset %u exceeds program size %zu",
+        root_offset,
+        program_data->size());


The new Minimal verification check only enforces an upper bound on the flatbuffer root table offset. Since this program format uses a FlatBuffers file_identifier (checked via ProgramBufferHasIdentifier), Minimal verification should also ensure the root offset cannot point into the header/identifier region (e.g., require root_offset >= sizeof(uoffset_t) + file identifier length, and that the buffer is at least that large). Without a lower-bound check, a corrupted root_offset like 4 can still pass this check and cause GetProgram() to interpret the identifier bytes as a table, risking out-of-bounds reads later during parsing.

runtime/executor/test/program_test.cpp

Add a preliminary check that the buffer is large enough to contain the flatbuffer header before reading the root offset, and ensure the root table position has room for at least a vtable offset.

The FlatBuffer root table offset was only validated when InternalConsistency verification was both requested and compiled in. With Minimal verification (the default), a malicious PTE file could set the root offset to point outside the buffer, causing OOB heap reads. Add a Minimal verification path that validates: - Lower bound: root_offset >= header size (offset + file identifier), preventing the root table from overlapping the header region. - Upper bound: root_offset <= size - sizeof(uoffset_t), ensuring room for at least the vtable offset at the root table position. Also return Error::NotSupported when InternalConsistency verification is requested but compiled out (ET_ENABLE_PROGRAM_VERIFICATION=0), instead of silently continuing with no verification. Addresses TOB-EXECUTORCH-12. 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.

You can also share your feedback on Copilot code review. Take the survey.

runtime/executor/program.cpp

+    // Minimal verification: check that the root table offset is within bounds.
+    // The first 4 bytes of the flatbuffer contain an offset to the root table.
+    ET_CHECK_OR_RETURN_ERROR(
+        program_data->size() >= sizeof(flatbuffers::uoffset_t),
+        InvalidProgram,
+        "Program data size %zu is too small for flatbuffer header",
+        program_data->size());
+    uint32_t root_offset = flatbuffers::ReadScalar<flatbuffers::uoffset_t>(


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 Feb 3, 2026

lucylq force-pushed the lfq.check-offset-minimal-verificaiton branch from 8fc7c5f to 3b4ce0c Compare February 3, 2026 01:05

lucylq marked this pull request as ready for review February 3, 2026 01:06

lucylq requested a review from JacobSzwejbka as a code owner February 3, 2026 01:06

Copilot AI review requested due to automatic review settings February 3, 2026 01:06

Copilot started reviewing on behalf of lucylq February 3, 2026 01:07 View session

Copilot AI reviewed Feb 3, 2026

View reviewed changes

runtime/executor/program.cpp Show resolved Hide resolved

runtime/executor/program.cpp Show resolved Hide resolved

lucylq requested a review from Copilot February 6, 2026 23:51

Copilot started reviewing on behalf of lucylq February 6, 2026 23:52 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

lucylq and others added 3 commits March 13, 2026 12:00

Check flatbuffer offset when verification is minimal

2120b59

Tighten root offset bounds check in minimal verification

1179e88

Add a preliminary check that the buffer is large enough to contain the flatbuffer header before reading the root offset, and ensure the root table position has room for at least a vtable offset.

Copilot AI review requested due to automatic review settings March 13, 2026 19:07

lucylq force-pushed the lfq.check-offset-minimal-verificaiton branch from be5ce97 to 13ba046 Compare March 13, 2026 19:07

lucylq mentioned this pull request Mar 13, 2026

Security #17738

Open

Copilot started reviewing on behalf of lucylq March 13, 2026 19:08 View session

Copilot AI reviewed Mar 13, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Check flatbuffer offset when verification is minimal#17129

Check flatbuffer offset when verification is minimal#17129
lucylq wants to merge 3 commits intomainfrom
lfq.check-offset-minimal-verificaiton

lucylq commented Feb 3, 2026 •

edited

Loading

Uh oh!

pytorch-bot bot commented Feb 3, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Feb 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

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

Uh oh!

Summary

Test plan

Uh oh!

pytorch-bot bot commented Feb 3, 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/17129

❌ 1 Awaiting Approval, 9 New Failures, 21 Pending

Uh oh!

github-actions bot commented Feb 3, 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!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

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!

2 participants

lucylq commented Feb 3, 2026 •

edited

Loading

pytorch-bot bot commented Feb 3, 2026 •

edited

Loading

This PR needs a `release notes:` label