Edit report at https://bugs.php.net/bug.php?id=61964&edit=1
ID: 61964
Comment by: nikic@php.net
Reported by: nikic@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] laruence@php.net
then I think we can simply prevent directory parameter
------------------------------------------------------------------------
[2012-05-09 23:37:07] felipe@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] laruence@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] nikic@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