Multiple memory safety issues in InitMOD / RenderMOD #3
Description
Hey, I was reading through the source code and noticed a few memory safety issues. Figured I'd report them in case MODPlay ever gets used with untrusted .mod files.
1. No buffer length parameter in InitMOD (most important one)
InitMOD takes const uint8_t *mod but doesn't know how big the buffer is. It immediately reads from offsets like mod[1080]–mod[1083] for the signature, mod[950] for the order count, and casts mod + 20 as sample headers. If someone passes in a truncated or very short file, this reads way past the end of the buffer.
A simple fix would be adding a uint32_t modlen parameter and validating that it's at least 1084 before accessing any fields.
2. Division by zero with FLT1 signature
If a MOD file has a FLT1 signature (1 channel), mp.channels gets set to 1. That passes the check on line 547 fine. But then in RenderMOD (line 428):
int32_t majorchmul = 131072 / (mp.channels / 2);Integer division: 1 / 2 = 0. Division by zero, instant crash.
Easy fix: reject channels < 2 (or at least check before dividing).
3. Sample data pointers can point past the file
samplemem (line 568) is calculated as mod + 1084 + pattern_data_size, and then each of the 31 sample data pointers is set from there. But there's no check that these pointers actually fall within the mod file's data. If a file claims to have 256 patterns and 31 channels, samplemem starts ~2MB into the buffer. If the file is actually much smaller, all the sample pointers are dangling — and the audio renderer reads from them.
4. Negative channel count via xCHN signature
If the first byte of the xCHN signature is less than ASCII '0' (0x30), then (signature >> 24) - '0' produces a negative number. The check channels == 0 || channels >= CHANNELS doesn't catch negatives (since e.g. -48 < 32), so it passes through. This messes up the samplemem calculation.
Fix: change the check to channels <= 0 || channels >= CHANNELS.
None of these are likely to be hit in normal usage with valid MOD files, but if MODPlay is ever used to load files from an untrusted source (user uploads, downloads, etc.), these could be triggered pretty easily. Happy to help with fixes if useful.