Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Cover offset+bounds wrapping in the APNG frame region check.#57025

Merged
auto-submit[bot] merged 1 commit intoflutter:mainfrom
bdero:bdero/apng-oob-wrapping
Dec 6, 2024
Merged

Cover offset+bounds wrapping in the APNG frame region check.#57025
auto-submit[bot] merged 1 commit intoflutter:mainfrom
bdero:bdero/apng-oob-wrapping

Conversation

@bdero
Copy link
Contributor

@bdero bdero commented Dec 6, 2024

The offset + bounds calculation in the bounds checks could wrap around, bypassing the check.

(Follow up to #56928)

if (
// Check for unsigned integer wrapping for
// frame.{x|y}_offset + frame_info.{width|height}().
frame.x_offset >
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this also needed in APNGImageGenerator::RenderDefaultImage?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, as there's no offset for the default image.

@bdero bdero marked this pull request as ready for review December 6, 2024 22:18
@bdero bdero added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 6, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Dec 6, 2024
@auto-submit
Copy link
Contributor

auto-submit bot commented Dec 6, 2024

auto label is removed for flutter/engine/57025, due to - The status or check suite Linux mac_android_aot_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

@bdero bdero added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 6, 2024
@auto-submit auto-submit bot merged commit 1e63abe into flutter:main Dec 6, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 7, 2024
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Dec 7, 2024
flutter/engine@de53ed5...1e63abe

2024-12-06 [email protected] Cover offset+bounds wrapping in the APNG
frame region check. (flutter/engine#57025)
2024-12-06 [email protected] Roll Skia from 0d94e966268b to
c9e9ce277b80 (3 revisions) (flutter/engine#57024)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a
human
is aware of the problem.

To file a bug in Flutter:
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
zanderso pushed a commit to zanderso/engine that referenced this pull request Dec 9, 2024
…#57025)

The `offset + bounds` calculation in the bounds checks could wrap around, bypassing the check.

(Follow up to flutter#56928)
auto-submit bot pushed a commit that referenced this pull request Dec 9, 2024
…tion surface. (#57062)

This cherry-pick PR includes:

#56928 followed by #57025

It supersedes #56978.

### Issue Link:
What is the link to the issue this cherry-pick is addressing?

Issue was reported over email.

### Changelog Description:
Explain this cherry pick in one line that is accessible to most Flutter developers. See [best practices](https://github.com/flutter/flutter/blob/main/docs/releases/Hotfix-Documentation-Best-Practices.md) for examples

Fixes an out-of-bounds memory write in APNG decoding.

### Impact Description:
What is the impact (ex. visual jank on Samsung phones, app crash, cannot ship an iOS app)? Does it impact development (ex. flutter doctor crashes when Android Studio is installed), or the shipping production app (the app crashes on launch)

Fixes an issue in which an untrusted malformed APNG image could cause out of bounds memory writes, crashing the app.

### Workaround:
Is there a workaround for this issue?

There is no workaround.

### Risk:
What is the risk level of this cherry-pick?

### Test Coverage:
Are you confident that your fix is well-tested by automated tests?

### Validation Steps:
What are the steps to validate that this fix works?

Attempt to load the APNG used in the tests in the PR.
nick9822 pushed a commit to nick9822/flutter that referenced this pull request Dec 18, 2024
…/engine#57025)

The `offset + bounds` calculation in the bounds checks could wrap around, bypassing the check.

(Follow up to flutter/engine#56928)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

affects: engine autosubmit Merge PR when tree becomes green via auto submit App

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants