mpruett / audiofile

Audio File Library
https://audiofile.68k.org/
GNU Lesser General Public License v2.1
154 stars 41 forks source link

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

Open Zzero00 opened 2 years ago

Zzero00 commented 2 years ago

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 commented 2 years ago

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

bastien-roucaries commented 9 months ago

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;