Potential Security Issues

[JamieSlome]

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]

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]

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]

This is spam.

[JamieSlome]

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]

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]

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

Let me know what you think.

Cheers! 🍰