Add FlatBuffers Verifier checks to Impeller asset loading#187878
Merged
auto-submit[bot] merged 1 commit intoJun 16, 2026
Merged
Add FlatBuffers Verifier checks to Impeller asset loading#187878auto-submit[bot] merged 1 commit into
auto-submit[bot] merged 1 commit into
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces FlatBuffers verification checks in RuntimeStage::DecodeRuntimeStages and ShaderArchive::Create using flatbuffers::Verifier to ensure the payload buffers are valid before they are parsed. There are no review comments, and I have no feedback to provide.
bb5e2cf to
8ffca5a
Compare
jason-simmons
approved these changes
Jun 15, 2026
RuntimeStage::DecodeRuntimeStages() and ShaderArchive::Create() both check the FlatBuffer file identifier but skip the Verifier step before calling GetRuntimeStages() / GetShaderArchive(). Without verification, a malformed .iplr or .shar payload can cause out-of-bounds reads when the FlatBuffer accessors follow corrupted offsets. Add flatbuffers::Verifier calls after the identifier check so that structurally invalid buffers are rejected before any field access. Add unit tests that construct buffers with valid file identifiers but corrupt root table offsets, confirming the Verifier rejects them.
8ffca5a to
b17353a
Compare
Contributor
|
autosubmit label was removed for flutter/flutter/187878, because - The status or check suite Mac mac_unopt has failed. Please fix the issues identified (or deflake) before re-applying this label. |
This was referenced Jun 16, 2026
auto-submit Bot
pushed a commit
to flutter/packages
that referenced
this pull request
Jun 16, 2026
flutter/flutter@5827d5f...3a0420c 2026-06-16 Rusino@users.noreply.github.com Implement font fallback (flutter/flutter#187520) 2026-06-16 amhurtado@protonmail.com Add FlatBuffers Verifier checks to Impeller asset loading (flutter/flutter#187878) 2026-06-16 engine-flutter-autoroll@skia.org Roll Packages from aa964a3 to 8286d39 (1 revision) (flutter/flutter#188067) 2026-06-16 engine-flutter-autoroll@skia.org Roll Skia from 9c2b83788409 to d7196b0b4939 (1 revision) (flutter/flutter#188066) 2026-06-16 engine-flutter-autoroll@skia.org Roll Skia from ef17057bb776 to 9c2b83788409 (1 revision) (flutter/flutter#188061) 2026-06-16 engine-flutter-autoroll@skia.org Roll Skia from 500025456bb5 to ef17057bb776 (1 revision) (flutter/flutter#188058) 2026-06-16 engine-flutter-autoroll@skia.org Roll Skia from cb1035ff14bf to 500025456bb5 (5 revisions) (flutter/flutter#188057) 2026-06-16 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from TbB86Po_HDe1dvXvT... to VeLhhlDcod09NR4Hb... (flutter/flutter#188055) 2026-06-16 engine-flutter-autoroll@skia.org Roll Skia from 70acf6a5e7c9 to cb1035ff14bf (3 revisions) (flutter/flutter#188054) 2026-06-16 41930132+hellohuanlin@users.noreply.github.com [pv]skip non-tappable web view workaround on ios 26.4 (flutter/flutter#185424) 2026-06-16 mdebbar@google.com [web] RenderParagraph needs paint after a DPR change (flutter/flutter#186968) 2026-06-16 30870216+gaaclarke@users.noreply.github.com Adds gamma correction to windows text. (flutter/flutter#187871) 2026-06-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Add a platform view test to android_hardware_smoke_test (#187913)" (flutter/flutter#188051) 2026-06-15 awolff@google.com Add a platform view test to android_hardware_smoke_test (flutter/flutter#187913) 2026-06-15 codefu@google.com feat: linux_analyze in a workflow (flutter/flutter#187889) 2026-06-15 mdebbar@google.com [web] Changes to WebParagraph configuration (flutter/flutter#187188) 2026-06-15 matt.boetger@gmail.com Fail gracefully on Android AVD lock errors during startup (flutter/flutter#187200) 2026-06-15 bkonyi@google.com [flutter_tools] Fix flakiness in widget_preview_detection_test (flutter/flutter#187938) 2026-06-15 jason-simmons@users.noreply.github.com Exclude fuchsia-sdk/sdk/.build-id from the builder cache archive (flutter/flutter#187826) 2026-06-15 engine-flutter-autoroll@skia.org Roll Skia from c8d9f80f13e4 to 70acf6a5e7c9 (4 revisions) (flutter/flutter#188020) 2026-06-15 engine-flutter-autoroll@skia.org Roll Packages from b78ad83 to aa964a3 (7 revisions) (flutter/flutter#188021) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC bmparr@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
via-guy
pushed a commit
to via-guy/flutter
that referenced
this pull request
Jun 26, 2026
…7878) ## Summary - Add `flatbuffers::Verifier` validation to `RuntimeStage::DecodeRuntimeStages()` and `ShaderArchive::Create()` before accessing FlatBuffer contents - Both functions already check the file identifier (`RuntimeStagesBufferHasIdentifier` / `ShaderArchiveBufferHasIdentifier`) but skip the structural verification step, meaning malformed `.iplr` or `.shar` payloads can cause out-of-bounds reads when FlatBuffer accessors follow corrupted offsets ## Context The FlatBuffers library provides a `Verifier` class specifically for validating buffer integrity before access. The Impeller runtime stage and shader archive loaders currently skip this step. Adding it ensures that structurally invalid buffers are rejected with a clear error before any field is dereferenced. ## Test plan - [ ] Existing `RuntimeStageTest::CanRejectInvalidBlob` continues to pass (already fills buffer with junk bytes — now rejected at the Verifier step rather than at later field access) - [ ] Existing `RuntimeStageTest::CanReadValidBlob` and shader archive tests remain green (valid buffers pass verification) - [ ] Manually verified that a truncated `.iplr` file is rejected with "buffer failed verification" rather than crashing 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
flatbuffers::Verifiervalidation toRuntimeStage::DecodeRuntimeStages()andShaderArchive::Create()before accessing FlatBuffer contentsRuntimeStagesBufferHasIdentifier/ShaderArchiveBufferHasIdentifier) but skip the structural verification step, meaning malformed.iplror.sharpayloads can cause out-of-bounds reads when FlatBuffer accessors follow corrupted offsetsContext
The FlatBuffers library provides a
Verifierclass specifically for validating buffer integrity before access. The Impeller runtime stage and shader archive loaders currently skip this step. Adding it ensures that structurally invalid buffers are rejected with a clear error before any field is dereferenced.Test plan
RuntimeStageTest::CanRejectInvalidBlobcontinues to pass (already fills buffer with junk bytes — now rejected at the Verifier step rather than at later field access)RuntimeStageTest::CanReadValidBloband shader archive tests remain green (valid buffers pass verification).iplrfile is rejected with "buffer failed verification" rather than crashing🤖 Generated with Claude Code