IMG_LoadTexture_IO read too much data, far beyond the end of the PNG data

[flowCRANE]

Continuation of PNG reading issues reported earlier: #360

If a binary file (or any stream) contains multiple PNG images, one after another, they cannot be loaded correctly using the IMG_LoadTexture_IO function, because the pointer in the stream is moved far beyond the IEND chunk of the image.

For example, I have a binary file sdl.dat containing two identical PNG images, one after the other. The image size is 10,982 bytes, there are two in the file, so the file size is 21,964 bytes. I open the file and try to load both images into two textures, like this:

var
  Stream: PWnc_SDL_Stream;
  Image1: PWnc_SDL_Texture;
  Image2: PWnc_SDL_Texture;
begin
  { initializing SDL, creating window and renderer etc. }

  Stream := SDL_IOFromFile('sdl.dat', 'rb');

  if Stream <> nil then
  begin
    Wnc_LogInfo(['Before:    ', SDL_TellIO(Stream)]);
    Image1 := IMG_LoadTexture_IO(Wnc_RendererGet(), Stream, False);

    if Image1 = nil then
      Wnc_LogError(['Filed to load 1st image: "', SDL_GetError(), '"']);

    Wnc_LogInfo(['After 1st: ', SDL_TellIO(Stream)]);
    Image2 := IMG_LoadTexture_IO(Wnc_RendererGet(), Stream, False);

    if Image2 = nil then
      Wnc_LogError(['Filed to load 2nd image: "', SDL_GetError(), '"']);

    if Image1 <> nil then SDL_DestroyTexture(Image1);
    if Image2 <> nil then SDL_DestroyTexture(Image2);

    if not SDL_CloseIO(Stream) then
      Wnc_LogError(['Failed to close file: "', SDL_GetError(), '"']);
  end
  else
    Wnc_LogError(['Failed to open file: "', SDL_GetError(), '"']);

  { finalizing SDL etc. }
end.

The first image loads correctly, but the second one doesn't. I see an error reading the second image in the console:

00  00:00:02.813  INFO     Before:    0
00  00:00:02.815  INFO     After 1st: 11094
00  00:00:02.816  ERROR    Filed to load 2nd image: "Unsupported image format"
00  00:00:02.816  INFO     After 2nd: 11094

A single image takes up 10,982 bytes, but after reading the first one from the stream, the stream pointer is moved to position 11,094, which means that the PNG decoder went beyond the IEND chunk and read 112 bytes too many. Because the stream pointer is in the wrong position, loading the second image is impossible (it is far beyond the header of the second image).

This problem does not exist when I read an image from a stream that is at the end of the stream — the decoder correctly stops reading after processing the IEND chunk, but only because there is no more data in the stream.

The same problem exists when I use the IMG_LoadTextureTyped_IO function:

Image1 := IMG_LoadTextureTyped_IO(Wnc_RendererGet(), Stream, False, 'PNG');

After loading the first image, the pointer in the stream is at position 11,094, instead of 10,982.

What is also interesting is that the literal specifying the file type, given in the last parameter of the IMG_LoadTextureTyped_IO function, has no significance, because no matter what I specify there (even some nonsense filetype like WTF or LOL), the first image will be loaded correctly anyway. For example:

Image1 := IMG_LoadTextureTyped_IO(Wnc_RendererGet(), Stream, False, 'BMP');

and the first image was loaded (same console logs as earlier), although I told it to load BMP, it loaded PNG. It looks like this parameter is being ignored by the SDL, which also looks like a bug.

I can load both images without any problems in Free Pascal using the standard classes:

var
  Stream: TFileStream;
  Image:  TPortableNetworkGraphic;
begin
  Stream := TFileStream.Create('sdl.dat', fmOpenRead);
  try
    Image := TPortableNetworkGraphic.Create();
    try
      try
        Image.LoadFromStream(Stream); // load first one
        Image.LoadFromStream(Stream); // load second one

        ShowMessage('Loading successfull');
      except
        on E: Exception do
          ShowMessage('Exception: ' + E.ClassName);
      end;
    finally
      Image.Free();
    end;
  finally
    Stream.Free();
  end;
end;

Images load correctly, I see the message Loading successful on the screen.

If the PNG decoder reads data from a stream and does not stop after processing the IEND chunk, not only does it prevent the stream data that is right after the image data from being loaded correctly, but it should also be considered a buffer overrun and a potential vulnerability. For more information and links, see this comment: #360 (comment)

The attachment contains an archive with two files: sdl.png is the image from which I built the binary file, and sdl.dat is the tested binary file, containing two sdl.png images stored one after another.

test files.zip