vgmrips / vgmplay-legacy

VGM file command-line player and Winamp plugin.
http://vgmrips.net
221 stars 52 forks source link

Potential Security Issues #81

Open JamieSlome opened 3 years ago

JamieSlome commented 3 years ago

Hello,

We recently received multiple vulnerability disclosures against your repository. I couldn't find an e-mail to contact or a security process to follow, so created this issue instead.

If you would like me to e-mail over the details or put them on the GitHub Issue, I'm more than happy to facilitate this for you. Otherwise, you can access the advisories here and here.

It is private to you and the discloser of the report.

If you have any questions, let me know.

-- Jamie from huntr.dev

superctr commented 3 years ago

Could you please put the issues here on Github?

Also, this repository is no longer actively supported- the new version is split among three repositories: https://github.com/ValleyBell/libvgm, https://github.com/ValleyBell/vgmplay-libvgm, https://github.com/ValleyBell/in_vgm-libvgm

kode54 commented 3 years ago

The owner of this repository must log into the above site with the requisite Github account to even see the reports. The discloser receives a bounty for each valid bug report. Apparently paid by that site for any arbitrary person who reports a so-called "0 day" against any Github repository.

vampirefrog commented 3 years ago

This is spam.

JamieSlome commented 3 years ago

https://huntr.dev/bounties/1-other-vgmrips/vgmplay/

✍️ Description

Hi, environement variables are copied into buffers using strcpy without prior size sanitization in https://github.com/vgmrips/vgmplay/blob/fb3a72d37b7e515e800eddbe776a17a3531cfa8b/VGMPlay/VGMPlayUI.c#L355. This can lead to a buffer overflow :

int main(int argc, char* argv[])
{
    int argbase;
    int ErrRet;
    char* AppName;
/**/
    char* AppPathPtr;
    const char* StrPtr;
    /**/
/**/
#ifndef WIN32
    // Path 3: home directory
    StrPtr = getenv("XDG_CONFIG_HOME");
    if (StrPtr != NULL && StrPtr[0] == '\0')
    {
        strcpy(AppPathPtr, StrPtr);//env variable XDG_CONFIG_HOME is copied using strcpy
    }
    else
    {
        StrPtr = getenv("HOME");
        if (StrPtr != NULL)
            strcpy(AppPathPtr, StrPtr);//env variable HOME is copied using strcpy
        else
            strcpy(AppPathPtr, "");
        strcat(AppPathPtr, "/.config");
    }
    /**/
#endif
}

🕵️‍♂️ Proof of Concept

Export a sufficiently long XDG_CONFIG_HOME or HOME variable in the environement (export XDG_CONFIG_HOME=$(python -c'print("A"*big_size)') for example) and run ./VGMPlay

💥 Impact

Crash, code execution for local attackers who have access to a local shell

JamieSlome commented 3 years ago

https://huntr.dev/bounties/2-other-vgmrips/vgmplay/

✍️ Description

Hi, a buffer overflow was found in https://github.com/vgmrips/vgmplay/blob/fb3a72d37b7e515e800eddbe776a17a3531cfa8b/VGMPlay/VGMPlayUI.c#L493

The executable ./VGMPlay.exe is accepting arguments and sometimes copy them into local buffers using strcpy and without checking the size of these arguments. One example below :

char VgmFileName[MAX_PATH];
/**/

int main(int argc, char* argv[])
{
    int argbase;
    /**/
    char* DispFileName;

/**/

    argbase = 0x01;
    /**/
    printf("\nFile Name:\t");
    if (argc <= argbase)//if 0 arguments are passed to the programs execute this branch
    {
        /**/
    }
    else
    {
        // The argument should already use the ANSI codepage.
        strcpy(VgmFileName, argv[argbase]);//copy argv[1] into .bss based VgmFileName variable
        DispFileName = GetLastDirSeparator(VgmFileName);
        if(DispFileName && strlen(DispFileName) > 2)
            DispFileName++;
        else
            DispFileName = VgmFileName;
        printf("%s\n", DispFileName);
    }
}

The program copies argv[1] directly into VgmFileName using strcpy leading to a buffer overflow.

🕵️‍♂️ Proof of Concept

To trigger the segmentation fault run the following command : ./VGMPlay.exe $(python -c'print("A"*300)').m3u

Without the .m3u extension the segmentation fault wouldn't have occured immediately, due to the program logic (I didn't perform a detailed debugging), as for the value 300, in Windows : MAX_PATH, which is the size of VgmFileName is 260.

💥 Impact

Segfault, code execution if successfully exploited

JamieSlome commented 3 years ago

@superctr - I have attached both reports above for you.

Let me know what you think.

Cheers! 🍰