[XM] OOB reads in mixer code

[sezero]

From a fuzz archive from Aug.2019: Files attached with valgrind outputs. Valgrind runs were made on an i686-linux using examples/test/test.c. An example output is inlined below from the 1st file, the rest of them are similar. @AliceLR: Need help with this.

Playing minimum (4 chn)
==29020== Invalid read of size 2
==29020==    at 0x8074CBE: Mix32StereoNormal (virtch.c:307)
==29020==    by 0x807728A: AddChannel (virtch.c:1073)
==29020==    by 0x807824A: VC1_WriteSamples (virtch.c:1203)
==29020==    by 0x80776D8: VC1_WriteBytes (virtch_common.c:278)
==29020==    by 0x8055F8E: VC_WriteBytes (virtch_common.c:161)
==29020==    by 0x8048D87: NS_Update (drv_nos.c:70)
==29020==    by 0x804966D: MikMod_Update (mdriver.c:311)
==29020==    by 0x8048CB6: main (test.c:84)
==29020==  Address 0x4042212 is 6 bytes before a block of size 84 alloc'd
==29020==    at 0x4006041: calloc (vg_replace_malloc.c:593)
==29020==    by 0x8048E18: MikMod_calloc (mmalloc.c:118)
==29020==    by 0x8077BB8: VC1_SampleLoad (virtch_common.c:403)
==29020==    by 0x8055F55: VC_SampleLoad (virtch_common.c:159)
==29020==    by 0x80495C8: MD_SampleLoad (mdriver.c:283)
==29020==    by 0x8055C40: DitherSamples (sloader.c:486)
==29020==    by 0x8055CFD: SL_LoadSamples (sloader.c:508)
==29020==    by 0x804BDD7: Player_LoadGeneric_internal (mloader.c:622)
==29020==    by 0x804BE8D: Player_LoadGeneric (mloader.c:647)
==29020==    by 0x804BF54: Player_LoadFP (mloader.c:675)
==29020==    by 0x804BFA9: Player_Load (mloader.c:689)
==29020==    by 0x8048C64: main (test.c:74)

102-xm.tar.gz

[AliceLR]

In the second module (with the louder sample), this seems to be triggered by the 3rd row of pattern 7 in channel 4, which looks like: 0: A#6 01 --- --- ... 3: A-8 110 g04 180

The actual cause is negative sample position indices underflowing sample buffers. This module doesn't have a bidi sample, so it shouldn't ever have negative sample positions, even temporarily. I suspect frequency overflow, will update.

[AliceLR]

Okay, no, it's much stupider than that. AddChannel in virtch.c bounds the current sample position against the loop end with == and this frequency generates a step value high enough to skip that value. 🤦‍♀️

edit: fixing this does NOT fix the first file, only the second. The first file uses a bidi loop, so it needs alternative bounding.