Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory-leak bug in printfileinfo, in printinfo.c #60

Open
Zzero00 opened this issue Feb 2, 2022 · 2 comments
Open

Memory-leak bug in printfileinfo, in printinfo.c #60

Zzero00 opened this issue Feb 2, 2022 · 2 comments

Comments

@Zzero00
Copy link

Zzero00 commented Feb 2, 2022

There exists one Memory-leak bug in printfileinfo, in printinfo.c, which allows an attacker to leak the address of heap or libc via a crafted file.
To reproduce with the attached poc file:
poc.zip

Heap address leak:
./sfinfo ./heapleak_poc.aiff

Result(See the output of Copyright):

$ ./sfinfo ./heapleak_poc.aiff
File Name      ./heapleak_poc.aiff
File Format    Audio Interchange File Format (aiff)
Data Format    unknown
Audio Data     0 bytes begins at offset 0 (0 hex)
               0 channel, -1 frames
Sampling Rate  0.00 Hz
Duration       -inf seconds
Copyright      C��U

Libc address leak:
./sfinfo ./libleak_poc.aiff

Result(See the output of Copyright):

$ ./sfinfo ./libleak_poc.aiff
File Name      ./libleak_poc.aiff
File Format    Audio Interchange File Format (aiff)
Data Format    unknown
Audio Data     0 bytes begins at offset 0 (0 hex)
               0 channel, -1 frames
Sampling Rate  0.00 Hz
Duration       -inf seconds
Copyright      Copyright 1991, (d��i

This vulnerability can be triggered anywhere the printfileinfo function is called, for example, sfconvert.

The poc.py will help you to calculate the address, which is test on Ubuntu 20.04, python2.

Usage of poc.py:

$ python2 poc.py heap
[+] Starting local process './sfinfo': pid 17868
[*] Process './sfinfo' stopped with exit code 0 (pid 17868)
[+] heap_leak:0x55b2425d4243
[+] heap_base:0x55b2425c2000
$ python2 poc.py lib
[+] Starting local process './sfinfo': pid 17920
[*] Process './sfinfo' stopped with exit code 0 (pid 17920)
[+] lib_leak:0x7f3d0cbf5428
[+] libaudiofile_base:0x7f3d0cbc9000
[+] libc_base:0x7f3d0c9bf000

The audiofile project is built with:

$ ./autogen.sh --disable-docs --prefix=OUTPUT_DIR
$ make
$ make install

Descrtption of the Vulnerability:

First, the printfileinfo function calls the copyrightstring function to get data:

//printfileinfo function, printinfo.c
bool printfileinfo (const char *filename){
...
char *copyright = copyrightstring(file);
	if (copyright)
	{
		printf("Copyright      %s\n", copyright);
		free(copyright);
	}
...
}

Second, the copyrightstring function obtains copyright information from the file and returns a string pointer:

//copyrightstring function, printinfo.c
static char *copyrightstring (AFfilehandle file){
...
int datasize = afGetMiscSize(file, miscids[i]);
		char *data = (char *) malloc(datasize);
		afReadMisc(file, miscids[i], data, datasize);
		copyright = data;
		break;
...
}

However, it forgets to use memset or zero bytes to prevent the Memory-Leak Vulnerability.
Most importantly, the attacker can control the length of the memcpy when copying the copyright string, in the afReadMisc function, in Miscellaneous.cpp:

//afWriteMisc function, Miscellaneous.cpp
int afWriteMisc (AFfilehandle file, int miscellaneousid, const void *buf, int bytes)
{
...
	int localsize = std::min(bytes,
		miscellaneous->size - miscellaneous->position);
	memcpy((char *) miscellaneous->buffer + miscellaneous->position,
		buf, localsize);
	miscellaneous->position += localsize;
	return localsize;
...
}
@carnil
Copy link

carnil commented Feb 22, 2022

It appears that a CVE has been assigned to this issue: CVE-2022-24599

@bastien-roucaries
Copy link

Fixed by:

commit 4d3238843385b9929d7a1ab9034a6fc13949c7b4
Author: Bastien Roucariès <rouca@debian.org>
Date:   Sat Nov 11 15:58:50 2023 +0000

    Fix CVE-2022-24599
    
    Memory-leak bug in printfileinfo, due to memcpy on an non allocated memory buffer
    with a user declared string.
    
    Fix it by calloc(declaredsize+1,1) that zeros the buffer and terminate by '\0'
    for printf
    
    Avoid also a buffer overflow by refusing to allocating more than INT_MAX-1.
    
    Before under valgrind:
    libtool --mode=execute valgrind --track-origins=yes  ./sfinfo heapleak_poc.aiff
    
    Duration       -inf seconds
    ==896222== Invalid read of size 1
    ==896222==    at 0x4846794: strlen (vg_replace_strmem.c:494)
    ==896222==    by 0x49246C8: __printf_buffer (vfprintf-process-arg.c:435)
    ==896222==    by 0x4924D90: __vfprintf_internal (vfprintf-internal.c:1459)
    ==896222==    by 0x49DE986: __printf_chk (printf_chk.c:33)
    ==896222==    by 0x10985C: printf (stdio2.h:86)
    ==896222==    by 0x10985C: printfileinfo (printinfo.c:134)
    ==896222==    by 0x10930A: main (sfinfo.c:113)
    ==896222==  Address 0x4e89bd1 is 0 bytes after a block of size 1 alloc'd
    ==896222==    at 0x48407B4: malloc (vg_replace_malloc.c:381)
    ==896222==    by 0x109825: copyrightstring (printinfo.c:163)
    ==896222==    by 0x109825: printfileinfo (printinfo.c:131)
    ==896222==    by 0x10930A: main (sfinfo.c:113)
    ==896222==
    Copyright      C
    
    After:
    Duration       -inf seconds
    Copyright      C

diff --git a/sfcommands/printinfo.c b/sfcommands/printinfo.c
index 60e6947..f5cf925 100644
--- a/sfcommands/printinfo.c
+++ b/sfcommands/printinfo.c
@@ -37,6 +37,7 @@
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <limits.h>
 
 static char *copyrightstring (AFfilehandle file);
 
@@ -147,7 +148,11 @@ static char *copyrightstring (AFfilehandle file)
 	int		i, misccount;
 
 	misccount = afGetMiscIDs(file, NULL);
-	miscids = (int *) malloc(sizeof (int) * misccount);
+	if(!misccount)
+		return NULL;
+	miscids = (int *) calloc(misccount, sizeof(int));
+	if(!miscids)
+		return NULL;
 	afGetMiscIDs(file, miscids);
 
 	for (i=0; i<misccount; i++)
@@ -159,13 +164,16 @@ static char *copyrightstring (AFfilehandle file)
 			If this code executes, the miscellaneous chunk is a
 			copyright chunk.
 		*/
-		int datasize = afGetMiscSize(file, miscids[i]);
-		char *data = (char *) malloc(datasize);
+		size_t datasize = afGetMiscSize(file, miscids[i]);
+		if(datasize >= INT_MAX -1 ) {
+			goto error;
+		}
+		char *data = (char *) calloc(datasize + 1, 1);
 		afReadMisc(file, miscids[i], data, datasize);
 		copyright = data;
 		break;
 	}
-
+error:
 	free(miscids);
 
 	return copyright;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants