milkytracker / MilkyTracker

An FT2 compatible music tracker
http://milkytracker.github.io/
Other
1.7k stars 160 forks source link

Fix About screen out-of-bounds read/crash caused by missing null terminator #255

Closed nyanpasu64 closed 2 years ago

nyanpasu64 commented 3 years ago

When building with ASAN enabled (CMake flags -DCMAKE_CXX_FLAGS=-fsanitize=address -DCMAKE_LINKER_FLAGS=-fsanitize=address), I get a crash in PPGraphics_32bpp_generic::drawString() dereferencing invalid memory in str. The method is called by AnimatedFXControl::paint printing textBuffer, which is a string that isn't properly null-terminated, causing drawString to read past the end of valid memory if the final byte is nonzero.

This PR writes a null terminator to the buffer so the message is printed properly. It seems to never be overwritten (because textBuffer has a fixed length), and I tested that the message is printed properly when it reaches the end and loops (though I shortened the message to reduce the wait time).