sezero / mikmod

Mikmod Sound System (mirror of git repo at https://sf.net/projects/mikmod/)
http://mikmod.sourceforge.net/
69 stars 21 forks source link

asylum loader bounds and sanity checks #60

Closed sezero closed 2 years ago

sezero commented 2 years ago

After merging #58, we replaced hard-coded numsamples, etc with values read from the module itself: Is the following patch correct? It changes the samples read for loop's upper boundary from 64 to numsamples, and updates sanity checks. @AliceLR: Please have a look.

 diff --git a/libmikmod/loaders/load_asy.c b/libmikmod/loaders/load_asy.c
index 22e3c7e..1a9b070 100644
--- a/libmikmod/loaders/load_asy.c
+++ b/libmikmod/loaders/load_asy.c
@@ -179,7 +179,7 @@ static int ConvertNote(MODNOTE *n)

    if (instrument) {
        /* if instrument does not exist, note cut */
-       if ((instrument > 31) || (!mh->samples[instrument - 1].length)) {
+       if ((instrument > mh->num_samples) || (!mh->samples[instrument - 1].length)) {
            UniPTEffect(0xc, 0);
            if (effect == 0xc)
                effect = effdat = 0;
@@ -301,8 +301,17 @@ static BOOL ASY_Load(BOOL curious)

    _mm_read_UBYTES(mh->positions, 256, modreader);

+   #ifdef MIKMOD_DEBUG
+   fprintf(stderr, "ASY: bpm=%d, spd=%d, numins=%d, numpat=%d\n",
+       mh->inittempo, mh->initspeed, mh->num_samples, mh->num_patterns);
+   #endif
+   if (!mh->initspeed || !mh->inittempo || mh->num_samples > 64) {
+       _mm_errno = MMERR_NOT_A_MODULE;
+       return 0;
+   }
+
    /* read samples headers*/
-   for (t = 0; t < 64; t++) {
+   for (t = 0; t < mh->num_samples; t++) {
        s = &mh->samples[t];

        _mm_fseek(modreader, 0x126 + (t*37), SEEK_SET);
@@ -323,10 +332,7 @@ static BOOL ASY_Load(BOOL curious)
        return 0;
    }

-   if (mh->num_samples > 64) {
-       _mm_errno = MMERR_NOT_A_MODULE;
-       return 0;
-   }
+   _mm_fseek(modreader, 37*(64-mh->num_samples), SEEK_CUR);

    /* set module variables */
    of.initspeed = mh->initspeed;
AliceLR commented 2 years ago

I think this patch should be OK.

sezero commented 2 years ago

Patch pushed as https://github.com/sezero/mikmod/commit/48598e75d3d5da1207dc9f27177f1bb990bdf7fc