CdtmLoader::rewind() assigns a default instrument to each
channel matching the channel number, so at least 9 instruments
are needed to prevent illegal memory accesses.

This fixes crashes for the following fuzzing samples:
  i-110_001.dtm, i-110_002.dtm, i-110_003.dtm, i-110_004.dtm,
  i-110_031.dtm, i-110_032.dtm, i-110_039.dtm, i-110_040.dtm,
  i-110_052.dtm, i-110_053.dtm, i-110_054.dtm, i-110_055.dtm,
  i-110_059.dtm, i-110_061.dtm, i-110_063.dtm, i-110_064.dtm,
  i-110_100.dtm, i-110_110.dtm, i-110_111.dtm, i-110_112.dtm,
  i-110_113.dtm, i-110_114.dtm, i-110_115.dtm, i-110_116.dtm,
  i-110_117.dtm, i-110_118.dtm, i-110_119.dtm, i-110_120.dtm,
  i-110_121.dtm, i-110_122.dtm, i-110_125.dtm, i-110_133.dtm.

Bug: adplug#110

Improve readability of the code:
* Make conversion tables static.
* Replace many literal numbers with sizeof or enums. Add a bunch
  of enums to the class in the header for that purpose.
* Use binistream::readString() instead of reading bytes in a loop.
* Declare loop counters locally in the loops.
* Make pattern a dtm_event[...][...] array with automatic storage.
  Cast it to (unsigned char *) only for unpack_pattern() and avoid
  all size and offset calculations.
* Add a variable to replace multiple copies of (event->byte1 & 15)
  during pattern conversion.
* Update track counter t along with j directly in for loop.
* Update formatting in a few places (mostly for loops; add spaces
  and move braces into the same line).
* Remove unneccessary input/output aliases in unpack_pattern().
* Use memset to repeat bytes for RLE decoding.

No functional changes.

* Refuse to load a file with zero patterns.
* Fail if an instrument name is too long.
* Ignore instrument events with invalid instrument number.
* Fail if order list contains invalid pattern numbers or
  restart position.

This commit fixes invalid memory accesses for the following
fuzzing samples:
  i-110_060.dtm, i-110_128.dtm.

Bug: adplug#110

Rework unpack_pattern() such that the unpacked pattern is
read directly from a file instead of from a memory buffer.
Also return a bool indicating success instead of the unpacked
length (which is known in advance). The complexity of the
method doesn't change much, but its usage becomes much simpler,
and the repeated buffer allocation/deallocation goes away.

No functional changes.

Lionel Debroux added another batch of fuzzing samples to issue adplug#110
yesterday. Hook up the 43 new samples in stresstest, too.

Crashes for the following new fuzzing samples have already been
fixed by the last few commits, though:
  i-110_139.dtm, i-110_143.dtm, i-110_164.dtm, i-110_165.dtm,
  i-110_166.dtm, i-110_176.dtm, i-110_178.dtm, i-110_179.dtm.

Bug: adplug#110
Reported-by: Lionel Debroux <lionel_debroux@yahoo.fr>

In Ca2mLoader::inputcode() and Ca2mLoader::uncompress(),
ibufcount should be compated to input_size, not MAXBUF.
Moreover, the former method had a type and compared the wrong
variable. Divide input_size by two to make it a count of
shorts, not bytes. Check for empty compressed input early.

Wrapping around the input/output buffer counters on overflow
still looks bogus. It would probably better to abort decoding
instead.

This commit fixes crashes for the following fuzzing samples:
  i-110_006.a2m, i-110_010.a2m, i-110_044.a2m, i-110_074.a2m,
  i-110_076.a2m, i-110_108.a2m, i-110_132.a2m, i-110_137.a2m,
  i-110_170.a2m.

i-110_008.a2m loads now, but doesn't finish to play in a
reasonable time frame; i-110_042.a2m and i-110_073.a2m hang
during load instead of crashing.

The following samples now crash in a different place:
  i-110_005.a2m, i-110_007.a2m, i-110_011.a2m, i-110_041.a2m,
  i-110_075.a2m, i-110_077.a2m, i-110_078.a2m, i-110_091.a2m.

Bug: adplug#110

Due to integer division with wrong rounding, one extra element
of len would be added to the sum when numpats was a multiple
of 8 (version >= 5) or 16. That was mostly benign, but for a
version >= 5 file with 64 patterns that would read past the
end of the len array. Fix the computation and simplify block
handling by introducing a variable holding the number of blocks.

While at it, simplify block decoding substantially by converting
the unrolled mess to a loop, fixing the misleading formatting
(or missin braces) along the way.

Also check that there is at least one pattern in the file.

This commit fixes an OOB read for fuzzing sample i-110_129.a2m.

Bug: adplug#110

Fix transposed digits in MAXDISTANCE. The correct value is
21839, not 21389. (That was hard to spot!) This typo could
cause k to wrap around to a value near 64k, way out of bounds
to access buf.

Also stop decoding when input or output buffers are exhausted
instead of wrapping around to the beginning to avoid possibly
endless hangs in such cases.

This commit fixes crashes and hangs for the following fuzzing
samples:
  i-110_005.a2m, i-110_009.a2m, i-110_038.a2m, i-110_041.a2m,
  i-110_042.a2m, i-110_045.a2m, i-110_073.a2m, i-110_107.a2m,
  i-110_124.a2m, i-110_126.a2m, i-110_138.a2m, i-110_157.a2m,
  i-110_159.a2m, i-110_168.a2m.

The following fuzzing samples now load successfully, but crash
or hang during playback:
  i-110_006.a2m, i-110_007.a2m, i-110_010.a2m, i-110_011.a2m,
  i-110_075.a2m, i-110_076.a2m, i-110_077.a2m, i-110_078.a2m,
  i-110_091.a2m.

Bug: adplug#110

Fuzzing samples may cause hangs, then stresstest will never
complete. Stop playback if we reach the one hour mark in a
song or kill the process if a sample runs for a minute real
time.

Also show a warning for invalid refresh values and improve
misleading wording when a sample doesn't load successfully.

Move the class members and methods used for decoding compressed
files to a new subclass. This subclass is only instantiated
in static method sixdepak::decode() (formerly sixdepak()), so
decompression data doesn't clutter the player class. Note that
the sixdepak subclass could easily moved out of Ca2mLoader class
and made local to the .cpp file or moved to its own header.

Remove all preprocessor constants and const members along the
way and replace them with enum constants. Replace the lookup
tables with static member functions (one of them is still
implemented with a lookup table, but the other two are simpler
and faster with arithmetic).

The old decode() method is renamed to do_decode() and returns
the decoded length now. The name is now used for the entry point,
and the code from the old sixdepak() partially went into the
constructor, do_decode(), and the new decode(). It allocates
a sixdepak instance dynamically, so buf became an array an is no
longer allocated separately.

Note that output_size is now unused. Future changes should use
it for the size of the destination buffer instead of hardcoded
MAXBUF.

No functional changes.

It's possible now to specify a length for the output buffer
passed to sixdepak::decode() (and the sixdepak constructor).
Change buffer lengths to size_t while at it.

Allocated buffer sizes have not changed yet.

It's known in advance how much data is needed when decoding
packed blocks. Allocate destination buffers for sixdepak::decode()
just large enough to hold the required amount of data. Decoding
will stop when the buffer is full and any excess data is ignored.

Using a sliding window buffer to copy from makes only sense
for streaming output. Since the sixdepak class writes its
output entirely to a memory buffer, we can copy directly
from the output buffer. Just remove the buf array and the
corresponding code from do_decode().
Also make obufcount a local variable since it's not used
outside of do_decode().

…l change)

* Add fixstringlength() helper that limits the length byte of
  pascal style strings to the available space. This seems to be
  useful for other players and may be used to a common utilities
  header later. Replace the manual checks.
* Replace some literal numbers with sizeof(...) or enum constants.
* Declare some variables where they are used first.
* Whitespace changes.

With some effects it's possible that setnote() is called with
note < 1. Clamp to 1 in that case. Simplify code a bit flow
while at it.

This fixes invalid array accesses of notetable with negative
index for the following fuzzing samples:
  i-110_006.a2m, i-110_007.a2m, i-110_010.a2m, i-110_075.a2m,
  i-110_076.a2m.

Bug: adplug#110

If all 64 orders are used, length is never initialized, leading
to a crash in CmodPlayer::rewind(). Fix that by adding the missing
initialization.

This fixes loading for the following fuzzing samples, but both
still crash during playback:
  i-110_016.cff, i-110_097.cff.

Bug: adplug#110

There is only enough space for 36 patterns. Trying to load more
will access memory past the end of the allocated arrays. Fix
that by adding a check that the number is valid (also rejecting
modules that are too short for the number of patterns or that
have no patterns).

This commit fixes OOB memory accesses while loading track data
for the following fuzzing samples:
  i-110_012.cff, i-110_013.cff, i-110_014.cff, i-110_015.cff,
  i-110_017.cff, i-110_018.cff, i-110_019.cff, i-110_020.cff,
  i-110_021.cff, i-110_022.cff, i-110_023.cff, i-110_024.cff,
  i-110_092.cff, i-110_093.cff, i-110_101.cff, i-110_102.cff,
  i-110_103.cff.

It also fixes the crashes in CmodPlayer::update() during playback
for i-110_016.cff and i-110_097.cff.

Bug: adplug#110

Don't even try to unpack the module if the recorded size is
less than the unpacker's header id. This should fix the issues
with fuzzing samples i-110_048.cff and i-110_105.cff that don't
reproduce in my setup (due to different ASAN version, probably).

Bug: adplug#110

* Issuing many 0x02 codepoints could increase code_length
  arbitrarily, leading to undefined behavior in get_code()
  and effectively ignoring large chunks of the input. Limit
  code_length to 16 bits, which is enough for a dictionary
  size of 0x8000. (We might choose to allow 32 bits instead;
  although not useful, RLE can use 32 bit counts anyway.)
* Fix undefined shifts in get_code() and prevent an overflow
  in unpack() for 32 bit RLE counts when long is 32 bit.
* Fail if RLE wants to repeat data beginning before the start
  of the output buffer.
* Handle invalid codes gracefully in translate_code().
* Avoid to overflow heap in expand_dictionary() (also prevents
  dictionary overflow since the heap is exhausted first).

4 bytes of padding ain't enough to reliably detect end of data
before reading past the end of the packed_module buffer. If the
data runs out while processing an RLE code, up to 32 bits for
repeat_counter an two more code points (up to 16 bits each) will
be read before unpacking terminates.

* Replace a few literal numbers with sizeof(...); there are still
  a lot of numbers that should be replaced.
* Change buffer allocation for module loading: allocate only
  header.size plus padding and swap buffers for packed files.
  Check for minimum module size before accessing data.
* Reorder some blocks/statements to clarify or simplify things,
  merge identical code, or eliminate branches.
* Simplify event pointer address calculation for track conversion
  by introducing a pointer to multi-dimensional array variable.
* Initialize and update track counter in for loops headers.
* Merge adjacent string literals.
* Shrink scope of variable definitions.
* Replace malloc/free with new/delete, use memcpy instead of open
  coded copy, drop unneccessary memset or use initialization instead.
* Use a switch statement in cff_unpacker::unpack(). Differentiate
  between success and failure by using different labels when jumping
  out of the surrounding loop.
* Remove dead stores to old_code. It's no longer used at all.
* Eliminate superfluous temporary string and double copy from
  cff_unpacker::translate_code().
* Spacing, line breaks, and indentation changes.
* Update a few comments.

…r_update()

The pointers to the channel's positions are updated directly
in the tune data in xadplayer_update(). This is a questionable
approach. Moreover, they are not reset to the original values
in xadplayer_rewind(), so rewinding doesn't work as expected.

Use a separate copy of channel pointers which is initialized
in xadplayer_rewind() and modified in xadplayer_update() to
rectify the situation.

* All arrays are sized for 9 channels, but the player uses only 8.
  Make the number (8) an enum constant and use that consistently
  (but leave the unused 9th row of the registers table unchanged).
* Convert psi.looping to a scalar, used as bitfield.
* Remove the lookup tables from the class interface. Declare them
  as static variables in the methods where they are used. Change
  their type to simplify access.
* In the xadplayer_update() loop, remove a nesting level by moving
  the delay check up and using continue.
* Make ptr a reference so it needn't be saved explicitly.
* Whitespace and comments changes.

…ords

Add a check for minimal tune size to xadplayer_load(), so
xadplayer_rewind() doesn't try to access data after the end
of tune. Also fix the end check in xadplayer_update().

This fixes crahes in xadplayer_rewind() for the following
fuzzing samples:
  i-110_034.xad, i-110_058.xad, i-110_123.xad, i-110_146.xad,
  i-110_161.xad, i-110_173.xad.

Bug: adplug#110

CxadratPlayer accesses and copies data without checking tune_size,
leading to OOB reads for truncated or manipulated files. Fix that
by checking tune_size before accessing data and rejecting files
that are too short.

While at it, also check whether the number of channels is valid.

This coommit fixes crashes for the fuzzing samples i-110_037.xad
and i-110_095.xad.

Bug: adplug#110

* Remove lookup tables and volume helper from class interface.
  Make the tables static local variables of xadplayer_update()
  and convert the helper method to a static function. All of
  them need to be moved to different places for that. Drop the
  rat prefix from their names, too.
* Use memcmp instead of strncmp in xadplayer_load() and simplify
  a memeset call in xadplayer_rewind().
* Copy a whole row at once when copying data to the "track" array
  in xadplayer_load(). This gets rid of one loop nesting level
  and reduces the number of memcpy() calls.
* Don't copy the current event in xadplayer_update(), but make
  event a reference to the array entry. Use new variable "pattern"
  to improve readability by avoiding nested array subscripts.
* Eliminate variable old_order_pos from effects processing by
  reordering some code.
* Whitespace and comment changes.

Lionel Debroux added a 3rd batch with 21 new fuzzing samples to
issue adplug#110 yesterday. Hook them up in stresstest, too. That brings
the total number of samples from this fuzzing round to 200.

Most of the crashes caused by the new samples have already been
fixed by previous commits. There are only two new samples still
crashing, i-110_188.mtk and i-110_197.sa2.

Bug: adplug#110
Reported-by: Lionel Debroux <lionel_debroux@yahoo.fr>

CxadflashPlayer doesn't check tune_size when it tries to access
data, so it can read out of bounds when setting instruments in
xadplayer_rewind() or when reading events during playback.

Check that tune_size isn't below the minimum in xadplayer_load()
and handle reaching the end of tune in xadplayer_update(). Ignore
"set instrument" commands with bad instrument number and handle
missing end marker in order list, too.

This commit fixes OOB reads for the fuzzing samples i-110_070.xad
and i-110_090.xad.

Bug: adplug#110

* Remove lookup tables from class interface and mark them static
  for the file.
* Convert flash_adlib_registers to a two-dimensional array to
  simplify indexing in the code.
* Remove flash_notes_encoded which is a simple division (by 12)
  table (with some unused entries at the end). Calculate quotient
  and remainder on the fly.
* Simplify control flow in xadplayer_update() a bit: Use continue
  statement in set instrument event branch to get rid of the big
  else block. Handle all effects in the switch statement; store
  frequency slide in a variable and apply it later to maintain
  correct oder; look up old channel frequency only when applying
  slide.
* Declare a pointer to the order list to clarify accesses to it.
* There is space for 128 instruments in the file, so change the
  return value of xadplayer_getinstruments() accordingly.
* Whitespace and formatting changes.

If the order list doesn't contain an end marker, length doesn't
get set at all. Fix that by using length as loop counter instead
of i.

While at it, also add a check for invalid pattern numbers in the
order list and ensure at least one pattern has been loaded.

This commit fixes OOB reads in rewind() for the following fuzzing
samples:
  i-110_047.sng, i-110_066.sng, i-110_080.sng, i-110_079.sng.

Bug: adplug#110

The number of patterns, order length, and restart position are
read from the file and used without checking. Add a check that
rejects the file if these values anr not in the expected range
to avoid OOB accesses or other unwanted behavior later.

Also the track data is read until the end of the file, possibly
overflowing the tracks array. Add checks to stop reading when
the allocated number of patterns is filled.

These changes fix crashes in rewind() for the follwing fuzzing
samples:
  i-110_049.sa2, i-110_050.sa2, i-110_067.sat, i-110_081.sa2,
  i-110_082.sa2, i-110_106.sat, i-110_134.sat, i-110_140.sat,
  i-110_142.sat, i-110_148.sa2, i-110_150.sa2, i-110_162.sa2,
  i-110_163.sa2, i-110_167.sa2, i-110_197.sa2,
and a crash in load() for sample i-110_149.sa2.

Bug: adplug#110

… gettitle()

If the order list contains an entry >= 64 it can lead to an OOB
access of the tracks array during playback. Add a check in load()
to prevent that (being more strict, enforce entries to be < nop).

Note that a similar check for track orders isn't necessary since
the number of allocated tracks is greater than the maximal value
of a byte.

* Simplify instruments loading, get rid of local insts struct.
* Fix spelling of "ARPEGGIO".
* Clean up headers (remove duplicate, prefer c++ headers).
* A few formatting changes.

Some of the fuzzing samples can be loaded by CmidPlayer, too.
So fix the problems that occur in that case as well.

Prevent accessing a lookup table with negative index and
improve excessively long running tests on broken files that
could require billions of update() calls to wait for the
next event:

* Ignore negative note values in midi_fm_playnote().
* Limit variable length numbers to allowed range.
* Fix wait calculation, initital value wasn't big enough.
* Limit track length to file size.

A position jump back to the current order entry doesn't set
the songend flag. Fix the off-by-one error to avoid playing
the pattern endlessly without ever reporting the end of the
song.

Also return always false in update() when speed is zero, as
songend is set only when speed changes to zero, but not if
it's already zero initially after rewind().

While uncompressing data in CmtkLoader::load(), memcpy() may
be called with overlapping source and destination buffers as
the byte count can be larger than the offset. The result of
such a call is undefined, so copy byte for byte in a for loop
instead.

Fixes: adplug#110

Don't load the whole file into memory before decompression, read
compressed data directly from the file. This simplifies the code
a bit and gets rid of a dynamic memory allocation.

Move a few bits around and add some gotos in the switch statement
to reduce code duplication.

Note that error checking is stricter than before. The size of the
decompressed data must match header.size exactly now. It could be
made more lenient if existing files need it.

* Remove patterns from struct mtkdata and split unused initial
  bytes from names. This simplifies size calculations and makes
  string copying clearer. (The type of patterns was wrong anyway.)
* Replace some literal numbers with sizeof(...).
* Don't use memset when setting the terminating NUL byte suffices.
* Whitespace changes.