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