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

IMG_LoadTexture_RW read too much data, far beyond the end of the PNG data #360

Open
flowCRANE opened this issue Apr 15, 2023 · 4 comments
Open

Comments

@flowCRANE
Copy link

The IMG_LoadTexture_RW function can load a PNG image saved in the stream, but if the stream contains some data after it (because it is, for example, a binary file with a lot of data), then after reading the PNG, the stream pointer is moved much too far, which further data could not be read correctly. The same problem occurs with the IMG_LoadTextureTyped_RW function.

The original discussion is here: https://discourse.libsdl.org/t/img-loadtexture-rw-seems-to-load-too-much-data

@flowCRANE flowCRANE changed the title IMG_LoadTexture_RW read too much data, far behind IEND chunk IMG_LoadTexture_RW read too much data, far beyond the end of the PNG data Apr 15, 2023
@slouken
Copy link
Collaborator

slouken commented Jan 31, 2025

You should not assume how much data will be read by an image decoder. If you are storing data in a stream you should have a directory of where each piece of data is located and skip to the next asset using that. A ZIP directory or RIFF chunk format has these properties.

@slouken slouken closed this as not planned Won't fix, can't repro, duplicate, stale Jan 31, 2025
@flowCRANE
Copy link
Author

flowCRANE commented Feb 1, 2025

This is not true. The format specification clearly states that the IEND chunk is the last chunk of the PNG image data stream, and is not followed by any data belonging to the PNG image data stream.

§ 15.3.1 Conformance of PNG datastreams

  1. The PNG datastream contains a PNG signature as the first content (see 5.2 PNG signature).
  2. With respect to the chunk types defined in this International Standard:
    • the PNG datastream contains as its first chunk, an IHDR chunk, immediately following the PNG signature;
    • the PNG datastream contains as its last chunk, an IEND chunk.
  3. No chunks or other content follow the IEND chunk.

Attempting to read data after an IEND chunk is an error, and this is exactly what happens in the IMG_LoadTexture_RW function in SDL2, and also in the IMG_LoadTexture_IO function in SDL3.

There is also a mention in the security section that data after the IEND chunk may contain malicious code, and while there is currently no evidence of it running on the victim's computer, reading data after the IEND chunk creates a vulnerability to this.

§ 13.3 Security considerations

A PNG datastream is composed of a collection of explicitly typed chunks. Chunks whose contents are defined by the specification could actually contain anything, including malicious code. Similarly there could be data after the IEND chunk which could contain anything, including malicious code. There is no known risk that such malicious code could be executed on the recipient's computer as a result of decoding the PNG image. However, a malicious application might hide such code inside an innocent-looking image file and then execute it.

For this reason, other decoders, including the one implemented in the Free Pascal runtime library, terminate reading on the IEND chunk. Reading PNG image data from a data stream after the IEND chunk is not only a bug prevents the correct reading of image data from the stream (which should be treated as buffer overflow), but also creates a vulnerability waiting to be exploited.

@slouken slouken reopened this Feb 1, 2025
@slouken slouken added this to the 3.2.2 milestone Feb 1, 2025
@slouken
Copy link
Collaborator

slouken commented Feb 1, 2025

That's a good point. Could you check which decoder is being used in your case and submit an upstream bug for that and reference it here, so we can pick up the fix when it happens?

@flowCRANE
Copy link
Author

Ok, I've added a new report for SDL3 related features — #524. However, since I'm using Free Pascal and SDL3 as a dynamically loaded library, I'm not able to provide more information about what SDL does internally.

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

No branches or pull requests

2 participants