Skip to content

Streamed EFF with a frame bigger than the first frame corrupts the heap #7883

Open

Description

@dustinspace217

Problem

A streamed EFF animation whose later frames are bigger than its first frame corrupts the heap and crashes the game. Found while testing #7882.

Reproduction

  1. Make an EFF of 20 uncompressed DDS frames, 256x128: frames 0-9 at 24 bits, frames 10-19 at 32 bits ($Type: dds, $Frames: 20).
  2. Put it in a mod's data/interface as 2_maincrane.eff with its frames, so it replaces that main hall animation.
  3. Start FSO with retail FS2 data at 1024x768 and wait in the main hall until the animation plays.

Expected vs actual

Expected: the animation plays, or the bad frames are refused with a warning.
Actual: the process aborts with double free or corruption (out). This happens on master on both OpenGL and Vulkan, in Debug builds, with or without #7882.

Mechanism

generic_render_eff_stream reuses one bitmap slot for every frame: bm_reload (bmpman.cpp:3061) only swaps the filename, so the slot keeps the first frame's size information. bm_lock_dds allocates mem_taken bytes from that first frame (bmpman.cpp:2278), but dds_read_bitmap reads as many bytes as the new file's own header says (ddsutils.cpp:446-450). A 32-bit frame after a 24-bit one writes w*h bytes past the buffer. A frame with larger dimensions or more mipmaps should do the same. That last part is from reading the code; I only ran the 24-to-32 case.

Fix alternatives

  • Pass the buffer size to dds_read_bitmap and refuse a frame whose data does not fit, with a warning.
  • Or, in bm_reload, re-read the new file's header and update the slot's size information.
  • Or, when an EFF starts streaming, compare every frame with the first and refuse a mismatched set up front. bm_load_animation already compares frames for non-streamed animations (bmpman.cpp:1857-1880), where it warns and stops using a texture array; it is safe there because each frame gets its own slot.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions