Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revisit the frame oob check #83

Merged
merged 3 commits into from
Sep 25, 2020
Merged

Revisit the frame oob check #83

merged 3 commits into from
Sep 25, 2020

Conversation

HeroicKatora
Copy link
Member

In particular, allow frames to be out-of-bounds even if the specification states the opposite. This is motivated by a test in image asserting such an image to be valid and most other decoders also recognizing it (though with slightly diverging results..). Turning it off effectively offloads all checks to the caller as was the case prior to 0.17.

At the same time, introduce a more immediate check for the memory allocated for frames when they are read into a new owned buffer. This fixes an issue where the true size could be off by a factor of 4.

The reservation of a frame buffer can take more bytes than specified by
the frame size, more specifically 4 times as many if we expand the
pixels. This previously escaped a check that was done in an unrelated
place and not protecting the actual allocation.
This restores the behaviour of previous version to NOT check it. This is
not valid according to strict reading of the specification but it may
appear in practice. In particular, imagemagick handles this in weird
ways that are not errors but silently adjust the width and height.. Not
sure about that style.
Copy link
Member Author

@HeroicKatora HeroicKatora left a comment

Choose a reason for hiding this comment

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

  1. Image Descriptor.
    a. Description. Each image in the Data Stream is composed of an Image
    Descriptor, an optional Local Color Table, and the image data. Each
    image must fit within the boundaries of the Logical Screen, as defined
    in the Logical Screen Descriptor.

I would also be interested in whether my reading is correct here.

src/reader/mod.rs Outdated Show resolved Hide resolved
@HeroicKatora HeroicKatora merged commit ddf74bb into master Sep 25, 2020
@HeroicKatora HeroicKatora deleted the retry-oob-check branch September 25, 2020 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant