mackron / dr_libs

Audio decoding libraries for C/C++, each in a single source file.
Other
1.24k stars 205 forks source link

dr_mp3 crash if the invalid file is loaded and uninit is called #253

Closed Youlean closed 1 year ago

Youlean commented 1 year ago

Maybe we can set data capacity to 0 here and check on uninit function in order to prevent the crash.

https://github.com/mackron/dr_libs/blob/056b6f5e1f7cc5046aac33e6331551b6b2813292/dr_mp3.h#L2849

Example:

static drmp3_bool32 drmp3_init_internal(drmp3* pMP3, drmp3_read_proc onRead, drmp3_seek_proc onSeek, void* pUserData, const drmp3_allocation_callbacks* pAllocationCallbacks)
{
    DRMP3_ASSERT(pMP3 != NULL);
    DRMP3_ASSERT(onRead != NULL);

    /* This function assumes the output object has already been reset to 0. Do not do that here, otherwise things will break. */
    drmp3dec_init(&pMP3->decoder);

    pMP3->onRead = onRead;
    pMP3->onSeek = onSeek;
    pMP3->pUserData = pUserData;
    pMP3->allocationCallbacks = drmp3_copy_allocation_callbacks_or_defaults(pAllocationCallbacks);

    if (pMP3->allocationCallbacks.onFree == NULL || (pMP3->allocationCallbacks.onMalloc == NULL && pMP3->allocationCallbacks.onRealloc == NULL)) {
        return DRMP3_FALSE;    /* Invalid allocation callbacks. */
    }

    /* Decode the first frame to confirm that it is indeed a valid MP3 stream. */
    if (drmp3_decode_next_frame(pMP3) == 0) {
        drmp3__free_from_callbacks(pMP3->pData, &pMP3->allocationCallbacks);    /* The call above may have allocated memory. Need to make sure it's freed before aborting. */
        pMP3->dataCapacity = 0;
        return DRMP3_FALSE; /* Not a valid MP3 stream. */
    }

    pMP3->channels   = pMP3->mp3FrameChannels;
    pMP3->sampleRate = pMP3->mp3FrameSampleRate;

    return DRMP3_TRUE;
}
DRMP3_API void drmp3_uninit(drmp3* pMP3)
{
    if (pMP3 == NULL) {
        return;
    }

#ifndef DR_MP3_NO_STDIO
    if (pMP3->onRead == drmp3__on_read_stdio) {
        FILE* pFile = (FILE*)pMP3->pUserData;
        if (pFile != NULL) {
            fclose(pFile);
            pMP3->pUserData = NULL; /* Make sure the file handle is cleared to NULL to we don't attempt to close it a second time. */
        }
    }
#endif

    if (pMP3->dataCapacity) {
      drmp3__free_from_callbacks(pMP3->pData, &pMP3->allocationCallbacks);
    }
}
mackron commented 1 year ago

To clarify, is drmp3_init_*() returning an error in this case? If it's returning false, you should not be calling drmp3_uninit().

Youlean commented 1 year ago

Indeed drmp3_init_file does return false. Other libs don't have problems if you call uninit after the file open fails and that confused me. Maybe to have assert on uninit to warn the user?

mackron commented 1 year ago

The correct usage is to not use the object at all if initialization fails, and that applies to all other dr_libs libraries (if it works in those, it's by coincidence, not by design). Setting this one to Will Not Fix.