You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Streamed EFF with a frame bigger than the first frame corrupts the heap #7883
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
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).
Put it in a mod's data/interface as 2_maincrane.eff with its frames, so it replaces that main hall animation.
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