Edit report at https://bugs.php.net/bug.php?id=61964&edit=1

 ID:                 61964
 Comment by:         ni...@php.net
 Reported by:        ni...@php.net
 Summary:            finfo_open with directory causes invalid free
 Status:             Open
 Type:               Bug
 Package:            Filesystem function related
 PHP Version:        master-Git-2012-05-06 (Git)
 Block user comment: N
 Private report:     N

 New Comment:

Reeze already has a patch which fixes this issue and several related memory 
leaks. Though I can't find it anywhere now :/


Previous Comments:
------------------------------------------------------------------------
[2012-05-13 10:56:10] larue...@php.net

then I think we can simply prevent directory parameter

------------------------------------------------------------------------
[2012-05-09 23:37:07] fel...@php.net

In fact the libmagic code seems not prepared to work with directory, even 
alloc'ing the data properly and freeing, it causes memleaks in other parts.

------------------------------------------------------------------------
[2012-05-06 14:06:07] larue...@php.net

diff --git a/ext/fileinfo/fileinfo.c b/ext/fileinfo/fileinfo.c
index 2c0e39a..9241e5b 100644
--- a/ext/fileinfo/fileinfo.c
+++ b/ext/fileinfo/fileinfo.c
@@ -315,16 +315,24 @@ PHP_FUNCTION(finfo_open)
        if (file_len == 0) {
                file = NULL;
        } else if (file && *file) { /* user specified file, perform 
open_basedir 
checks */
+               struct stat sb;
+
                if (strlen(file) != file_len) {
                        FILEINFO_DESTROY_OBJECT(object);
                        RETURN_FALSE;
                }
+
+               if (VCWD_STAT(file, &sb) || !S_ISREG(sb.st_mode)) {
+                       FILEINFO_DESTROY_OBJECT(object);
+                       RETURN_FALSE;
+               }
+
                if (!VCWD_REALPATH(file, resolved_path)) {
                        FILEINFO_DESTROY_OBJECT(object);
                        RETURN_FALSE;
                }
-               file = resolved_path;
 
+               file = resolved_path;
 #if PHP_API_VERSION < 20100412
                if ((PG(safe_mode) && (!php_checkuid(file, NULL, 
CHECKUID_CHECK_FILE_AND_DIR))) || php_check_open_basedir(file TSRMLS_CC)) {
 #else

------------------------------------------------------------------------
[2012-05-06 13:39:12] ni...@php.net

Description:
------------
The simple script

<?php
finfo_open(FILEINFO_NONE, '.');

causes an invalid free to be reported by glibc, with the following gdb bt:

#0  0x00130416 in __kernel_vsyscall ()
#1  0x009edc8f in __GI_raise (sig=6)
    at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#2  0x009f12b5 in __GI_abort () at abort.c:92
#3  0x00a2515c in __libc_message (do_abort=2, 
    fmt=0xafe4c0 "*** glibc detected *** %s: %s: 0x%s ***\n")
    at ../sysdeps/unix/sysv/linux/libc_fatal.c:189
#4  0x00a2ff22 in malloc_printerr (action=<optimized out>, 
    str=<optimized out>, ptr=0xbfff996c) at malloc.c:6283
#5  0x00a30168 in munmap_chunk (p=<optimized out>) at malloc.c:3540
#6  0x081ffde1 in apprentice_load (ms=0xb717d8f8, magicp=0xbfffa9bc, 
    nmagicp=0xbfffa9c0, fn=0xb717d150 "/home/nikic/dev/Phuzzy/results", 
    action=0) at /home/nikic/dev/php-src/ext/fileinfo/libmagic/apprentice.c:814
#7  0x081fecc9 in apprentice_1 (ms=0xb717d8f8, 
    fn=0xb717d150 "/home/nikic/dev/Phuzzy/results", action=0, mlist=0xb717d1a0)
    at /home/nikic/dev/php-src/ext/fileinfo/libmagic/apprentice.c:275
#8  0x081fef8c in file_apprentice (ms=0xb717d8f8, 
    fn=0xb717d150 "/home/nikic/dev/Phuzzy/results", action=0)
    at /home/nikic/dev/php-src/ext/fileinfo/libmagic/apprentice.c:369
#9  0x0820975e in magic_load (ms=0xb717d8f8, 
    magicfile=0xbfffaaec "/home/nikic/dev/Phuzzy/results")
    at /home/nikic/dev/php-src/ext/fileinfo/libmagic/magic.c:308
#10 0x081fdc23 in zif_finfo_open (ht=2, return_value=0xb717c2cc, 
    return_value_ptr=0x0, this_ptr=0x0, return_value_used=0, tsrm_ls=0x8c0f070)
    at /home/nikic/dev/php-src/ext/fileinfo/fileinfo.c:345
#11 0x085cf628 in zend_do_fcall_common_helper_SPEC (execute_data=0xb716007c, 
    tsrm_ls=0x8c0f070) at /home/nikic/dev/php-src/Zend/zend_vm_execute.h:642
#12 0x085d6ddb in ZEND_DO_FCALL_SPEC_CONST_HANDLER (execute_data=0xb716007c, 
    tsrm_ls=0x8c0f070) at /home/nikic/dev/php-src/Zend/zend_vm_execute.h:2219
#13 0x085cd8d5 in execute (op_array=0xb717cc14, tsrm_ls=0x8c0f070)
    at /home/nikic/dev/php-src/Zend/zend_vm_execute.h:410
#14 0x0859202e in zend_execute_scripts (type=8, tsrm_ls=0x8c0f070, retval=0x0, 
    file_count=3) at /home/nikic/dev/php-src/Zend/zend.c:1272
#15 0x084f4e91 in php_execute_script (primary_file=0xbfffe110, 
    tsrm_ls=0x8c0f070) at /home/nikic/dev/php-src/main/main.c:2473
#16 0x086dcbc9 in do_cli (argc=2, argv=0xbffff3b4, tsrm_ls=0x8c0f070)
    at /home/nikic/dev/php-src/sapi/cli/php_cli.c:988
#17 0x086de0ed in main (argc=2, argv=0xbffff3b4)
    at /home/nikic/dev/php-src/sapi/cli/php_cli.c:1361

The invalid free occurs in 
https://github.com/php/php-src/blob/master/ext/fileinfo/libmagic/apprentice.c#L814.

The code for loading from a directory seems completely broken: The filenames 
are snprintf'd into the mfn variable, which is a char[MAXPATHLEN].

For every file that variable is then written into the array, without further 
copying: filearr[files++] = mfn;

So basically at the end filearr just contains the last scanned path multiple 
times.

In the second loop the individual filearr elements are then freed, which is 
wrong in two senses: a) it's always the same array element, so it would be a 
multi-free b) mfn is an array, it was never allocated, so it shouldn't be freed.

The fix should be to copy mfn into a separate pointer when doing 
filearr[files++] = mfn;

:)



------------------------------------------------------------------------



-- 
Edit this bug report at https://bugs.php.net/bug.php?id=61964&edit=1

Reply via email to