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

Conversation

@bdero
Copy link
Member

@bdero bdero commented Dec 3, 2024

As per the spec:

The frame must be rendered within the region defined by x_offset, y_offset, width, and height. This region may not fall outside of the default image; thus x_offset plus width must not be greater than the IHDR width; similarly y_offset plus height must not be greater than the IHDR height.

Copy link
Member

@zanderso zanderso left a comment

Choose a reason for hiding this comment

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

lgtm w/ test

@bdero bdero marked this pull request as ready for review December 4, 2024 21:41
@bdero bdero changed the title Drop APNG frames that don't fit into the canvas. Drop APNG frames that don't fit entirely within the destination surface. Dec 4, 2024
@bdero
Copy link
Member Author

bdero commented Dec 4, 2024

There's another test with a malformed APNG I'm trying to work out a fix for.

@bdero
Copy link
Member Author

bdero commented Dec 5, 2024

It wasn't malformed. I was using an invalid offset to check the default frame destination region. :^)

@bdero bdero added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 5, 2024
@auto-submit auto-submit bot merged commit b4f52b2 into flutter:main Dec 5, 2024
34 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 5, 2024
@zanderso zanderso added the cp: beta cherry pick to the beta release candidate branch label Dec 5, 2024
flutteractionsbot pushed a commit to flutteractionsbot/engine that referenced this pull request Dec 5, 2024
…ce. (flutter#56928)

As per the [spec](https://www.w3.org/TR/png/#fcTL-chunk):

> The frame must be rendered within the region defined by x_offset, y_offset, width, and height. This region may not fall outside of the default image; thus x_offset plus width must not be greater than the [IHDR](https://www.w3.org/TR/png/#11IHDR) width; similarly y_offset plus height must not be greater than the [IHDR](https://www.w3.org/TR/png/#11IHDR) height.
auto-submit bot pushed a commit that referenced this pull request Dec 6, 2024
The `offset + bounds` calculation in the bounds checks could wrap around, bypassing the check.

(Follow up to #56928)
zanderso pushed a commit to zanderso/engine that referenced this pull request Dec 9, 2024
…ce. (flutter#56928)

As per the [spec](https://www.w3.org/TR/png/#fcTL-chunk):

> The frame must be rendered within the region defined by x_offset, y_offset, width, and height. This region may not fall outside of the default image; thus x_offset plus width must not be greater than the [IHDR](https://www.w3.org/TR/png/#11IHDR) width; similarly y_offset plus height must not be greater than the [IHDR](https://www.w3.org/TR/png/#11IHDR) height.
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
…ce. (flutter/engine#56928)

As per the [spec](https://www.w3.org/TR/png/#fcTL-chunk):

> The frame must be rendered within the region defined by x_offset, y_offset, width, and height. This region may not fall outside of the default image; thus x_offset plus width must not be greater than the [IHDR](https://www.w3.org/TR/png/#11IHDR) width; similarly y_offset plus height must not be greater than the [IHDR](https://www.w3.org/TR/png/#11IHDR) height.
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 cp: beta cherry pick to the beta release candidate branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants