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

Handle TGA images with packets that cross scan lines #6087

Merged
merged 2 commits into from Feb 27, 2022

Conversation

radarhere
Copy link
Member

Resolves #5729

https://en.wikipedia.org/wiki/Truevision_TGA

The older version of the TGA file format specification taken from the Appendix C of the Truevision Technical Guide states that run-length encoded (RLE) packets may cross scan lines: "For the run length packet, the header is followed by a single color value, which is assumed to be repeated the number of times specified in the header. The packet may cross scan lines (begin on one line and end on the next)".

However, page 24 of the TGA v2.0 specification states the exact opposite: "Run-length Packets should never encode pixels from more than one scan line. Even if the end of one scan line and the beginning of the next contain pixels of the same value, the two should be encoded as separate packets. In other words, Run-length Packets should not wrap from one line to another".

Consequently TGA readers need to be able to handle RLE data packets that cross scan lines since this was part of the original specification.

The image from the issue has these packets that crosses scan lines. I expect that we haven't dealt with this before because it is following an older specification. Currently, Pillow throws a buffer overrun error when handling this. At first, I was skeptical about removing a buffer overrun error, but then realised that it had been present since PIL, and #2241 removed the equivalent error for SUN images. The C changes here are modelled after #2241.

In other words, too much image data is found for a single row, and so this PR uses extra_bytes and extra_data to store that, until it can be looped over later.

The test image I have included was created by Pillow. Since Pillow plays it appropriately safe and does not cross scan lines, I then hexeditted it to test the relevant code.

Relevant documentation can be found at http://www.paulbourke.net/dataformats/tga/. Look at the "Image Data Field" under "DATA TYPE 10: Run Length Encoded, RGB images". The documentation also lead me to my first commit, to simplify reading two numbers from the file that are detailed the same way in the specification.

@radarhere radarhere mentioned this pull request Feb 23, 2022
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.

TGA buffer overrun
2 participants