Security: Negative channel count OOB read + division by zero in MOD parser

[ByamB4]

Summary

Found 3 vulnerabilities in the MOD file parser that can be triggered by crafted .mod files. All stem from insufficient validation of file-controlled values.

Vulnerabilities

1. Negative Channel Count via Crafted Signature → OOB Memory Access (CWE-681/CWE-125) — Critical

Location: InitMOD(), lines 531-548

The signature parsing computes channels by subtracting ASCII '0' (48) from a byte value:

// FLTx
if((signature & 0xFFFFFF00) == 0x464C5400) {
    channels = (signature & 0xFF) - '0';
}
// xCHN
if((signature & 0x00FFFFFF) == 0x0043484E) {
    channels = (signature >> 24) - '0';
}

If the byte is less than '0' (e.g., 0x00), channels becomes negative (e.g., -48). The validation check:

if(channels == 0 || channels >= CHANNELS) {
    return NULL;
}

Does NOT catch negative values since -48 is neither 0 nor >= 32 (CHANNELS).

With negative channels, the sample memory pointer at line 568 computes a massive negative offset:

const int8_t *samplemem = ((const int8_t *) mod) + 1084 + 64 * 4 * mp.channels * mp.maxpattern;

E.g., channels=-48, maxpattern=256 → offset = 1084 - 3,145,728 = points ~3MB before the buffer. All 31 sample data pointers are set to addresses before the MOD file buffer, causing OOB reads during playback.

Trigger: Craft a MOD file with signature FLT\x00 at offset 1080.

2. Division by Zero in RenderMOD When channels=1 (CWE-369)

Location: RenderMOD(), lines 428-429

int32_t majorchmul = 131072 / (mp.channels / 2);
int32_t minorchmul = 131072 / 3 / (mp.channels / 2);

With odd channel counts (particularly channels=1), mp.channels / 2 evaluates to 0 via integer division, causing a division by zero.

Trigger: Craft a MOD file with signature FLT1 or 1CHN at offset 1080.

3. Division by Zero in Period Calculation (CWE-369)

Location: ProcessMOD(), line 388

if(mp.ch[i].period)
    mp.ch[i].samplegen.period = mp.paularate / (mp.ch[i].period + (mp.ch[i].vibrato.val >> 7));

The code checks period != 0 but not the full divisor. With a small period (reduced by portamento effects) and vibrato depth of 15, the vibrato offset can negate the period exactly, making the divisor 0.

Proof of Concept (Bug 1)

import struct

mod = bytearray(2048)
mod[0:20] = b'Crafted MOD file\x00\x00\x00\x00'
mod[950] = 1   # 1 order
mod[952] = 0   # order 0 -> pattern 0
mod[1080:1084] = b'FLT\x00'  # channels = 0x00 - 0x30 = -48
with open('crash_negative_channels.mod', 'wb') as f:
    f.write(mod)

Suggested Fixes

Bug 1 — Change channel validation to reject negative values:

if(channels < 1 || channels >= CHANNELS) {
    return NULL;
}

Bug 2 — Guard the division:

int32_t half_channels = mp.channels / 2;
if(half_channels < 1) half_channels = 1;
int32_t majorchmul = 131072 / half_channels;

Bug 3 — Check the full divisor:

int32_t divisor = mp.ch[i].period + (mp.ch[i].vibrato.val >> 7);
if(divisor > 0)
    mp.ch[i].samplegen.period = mp.paularate / divisor;
else
    mp.ch[i].samplegen.period = 0;

Impact

• Bug 1: Heap/stack OOB read → information disclosure or crash
• Bugs 2 & 3: Crash (SIGFPE/exception) → denial of service

Found through manual source code audit. Reported responsibly.