php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #61964 finfo_open with directory causes invalid free
Submitted: 2012-05-06 13:39 UTC Modified: 2012-07-15 01:54 UTC
From: nikic@php.net Assigned: stas (profile)
Status: Closed Package: Filesystem function related
PHP Version: master-Git-2012-05-06 (Git) OS:
Private report: No CVE-ID: None
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: nikic@php.net
New email:
PHP Version: OS:

 

 [2012-05-06 13:39 UTC] 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;

:)


Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2012-05-06 14:06 UTC] 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-09 23:37 UTC] 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-13 10:56 UTC] laruence@php.net
then I think we can simply prevent directory parameter
 [2012-05-13 11:03 UTC] nikic@php.net
Reeze already has a patch which fixes this issue and several related memory leaks. Though I can't find it anywhere now :/
 [2012-05-24 18:00 UTC] reeze dot xia at gmail dot com
Hi, 
I've sent the file lib author, he replied it was intend to support
dir open. and I sent a Pull Request: https://github.com/php/php-src/pull/91

@felipe will you take look?

Thanks.
 [2012-05-24 18:25 UTC] felipe@php.net
Hi, I got a crash when running the following code in the php-src root dir using your patch (pull-request):

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

(gdb) r ../bug.php 
Starting program: /home/felipe/dev/php5_3/sapi/cli/php ../bug.php
[Thread debugging using libthread_db enabled]

Program received signal SIGSEGV, Segmentation fault.
memset () at ../sysdeps/i386/i686/memset.S:85
85	../sysdeps/i386/i686/memset.S: No such file or directory.
	in ../sysdeps/i386/i686/memset.S
Current language:  auto
The current source language is "auto; currently asm".
(gdb) bt
#0  memset () at ../sysdeps/i386/i686/memset.S:85
#1  0x081b29b9 in parse (ms=0x89baeac, mentryp=0xbfffae6c, nmentryp=0xbfffae68, line=0xbfff7dcb "> Makefile.fragments", lineno=143, action=0)
    at /home/felipe/dev/php5_3/ext/fileinfo/libmagic/apprentice.c:1178
#2  0x081b1a55 in load_1 (ms=0x89baeac, action=0, fn=0x89d4e48 "/home/felipe/dev/php5_3/acinclude.m4", errs=0xbfffae70, marray=0xbfffae6c, marraycount=0xbfffae68)
    at /home/felipe/dev/php5_3/ext/fileinfo/libmagic/apprentice.c:733
#3  0x081b1df8 in apprentice_load (ms=0x89baeac, magicp=0xbfffaef0, nmagicp=0xbfffaeec, fn=0x89baf58 "/home/felipe/dev/php5_3", action=0)
    at /home/felipe/dev/php5_3/ext/fileinfo/libmagic/apprentice.c:812
#4  0x081b0ebd in apprentice_1 (ms=0x89baeac, fn=0x89baf58 "/home/felipe/dev/php5_3", action=0, mlist=0x89bb33c) at /home/felipe/dev/php5_3/ext/fileinfo/libmagic/apprentice.c:275
#5  0x081b1180 in file_apprentice (ms=0x89baeac, fn=0x89baf58 "/home/felipe/dev/php5_3", action=0) at /home/felipe/dev/php5_3/ext/fileinfo/libmagic/apprentice.c:369
#6  0x081bb662 in magic_load (ms=0x89baeac, magicfile=0xbfffafb4 "/home/felipe/dev/php5_3") at /home/felipe/dev/php5_3/ext/fileinfo/libmagic/magic.c:308
#7  0x081b003d in zif_finfo_open (ht=2, return_value=0x89ba6b4, return_value_ptr=0x0, this_ptr=0x0, return_value_used=0, tsrm_ls=0x8874050)
    at /home/felipe/dev/php5_3/ext/fileinfo/fileinfo.c:350
#8  0x084673bd in zend_do_fcall_common_helper_SPEC (execute_data=0x89e924c, tsrm_ls=0x8874050) at /home/felipe/dev/php5_3/Zend/zend_vm_execute.h:320
#9  0x0846bac4 in ZEND_DO_FCALL_SPEC_CONST_HANDLER (execute_data=0x89e924c, tsrm_ls=0x8874050) at /home/felipe/dev/php5_3/Zend/zend_vm_execute.h:1640
---Type <return> to continue, or q <return> to quit---
#10 0x08466656 in execute (op_array=0x89badec, tsrm_ls=0x8874050) at /home/felipe/dev/php5_3/Zend/zend_vm_execute.h:107
#11 0x08433ee9 in zend_execute_scripts (type=8, tsrm_ls=0x8874050, retval=0x0, file_count=3) at /home/felipe/dev/php5_3/Zend/zend.c:1236
#12 0x083ae512 in php_execute_script (primary_file=0xbffff434, tsrm_ls=0x8874050) at /home/felipe/dev/php5_3/main/main.c:2308
#13 0x08510211 in main (argc=2, argv=0xbffff5b4) at /home/felipe/dev/php5_3/sapi/cli/php_cli.c:1189
(gdb) f 1
#1  0x081b29b9 in parse (ms=0x89baeac, mentryp=0xbfffae6c, nmentryp=0xbfffae68, line=0xbfff7dcb "> Makefile.fragments", lineno=143, action=0)
    at /home/felipe/dev/php5_3/ext/fileinfo/libmagic/apprentice.c:1178
1178			(void)memset(m, 0, sizeof(*m));
Current language:  auto
The current source language is "auto; currently c".
(gdb) p m
$1 = (struct magic *) 0x1d0
 [2012-05-25 02:03 UTC] laruence@php.net
libmagic is obviously support dir,  but PHP is not, I think you asked the wrong 
guy and wrong question.

what I mean is,  libmagic need a big operation, not just one and one little 
suture, before this, we can just be consistent with the doc said : 'no directory 
supproted" 

thanks
 [2012-05-25 11:07 UTC] reeze dot xia at gmail dot com
Hi, 
  @Felipe I've updated the patch, I made a simple reproducable magic file:
string
> A
> B

and I add it to the test file.(file command itself will leak when use this magic 
file).

@Laruence
the reason why I ask Christos Zoulas is you mentioned load multiple magic file 
may 
lead problem, then I asked him.

Anyway, the crash is a problem of finfo and the fprintf thing, maybe more need 
to be done.
 [2012-07-15 01:53 UTC] stas@php.net
Automatic comment on behalf of reeze.xia@gmail.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=1d2f61904987133d542c68cd349cf313d0bef1c8
Log: Fixed bug #61964 (finfo_open with directory cause invalid free)
 [2012-07-15 01:54 UTC] stas@php.net
-Status: Open +Status: Closed -Assigned To: +Assigned To: stas
 [2012-07-15 01:54 UTC] stas@php.net
This bug has been fixed in SVN.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.

 For Windows:

http://windows.php.net/snapshots/
 
Thank you for the report, and for helping us make PHP better.


 [2012-10-18 15:31 UTC] laruence@php.net
Automatic comment on behalf of laruence
Revision: http://git.php.net/?p=php-src.git;a=commit;h=0d7965f0a32dc967a1665d274cdc0e39d44c87bb
Log: Merge the fix for #61964 to 5.3, which will fix #63304
 [2012-10-18 15:32 UTC] laruence@php.net
Automatic comment on behalf of laruence
Revision: http://git.php.net/?p=php-src.git;a=commit;h=0d7965f0a32dc967a1665d274cdc0e39d44c87bb
Log: Merge the fix for #61964 to 5.3, which will fix #63304
 [2012-10-18 15:34 UTC] laruence@php.net
Automatic comment on behalf of laruence
Revision: http://git.php.net/?p=php-src.git;a=commit;h=0d7965f0a32dc967a1665d274cdc0e39d44c87bb
Log: Merge the fix for #61964 to 5.3, which will fix #63304
 [2014-10-07 23:21 UTC] stas@php.net
Automatic comment on behalf of laruence
Revision: http://git.php.net/?p=php-src-security.git;a=commit;h=0d7965f0a32dc967a1665d274cdc0e39d44c87bb
Log: Merge the fix for #61964 to 5.3, which will fix #63304
 [2014-10-07 23:25 UTC] stas@php.net
Automatic comment on behalf of reeze.xia@gmail.com
Revision: http://git.php.net/?p=php-src-security.git;a=commit;h=1d2f61904987133d542c68cd349cf313d0bef1c8
Log: Fixed bug #61964 (finfo_open with directory cause invalid free)
 [2014-10-07 23:32 UTC] stas@php.net
Automatic comment on behalf of laruence
Revision: http://git.php.net/?p=php-src-security.git;a=commit;h=0d7965f0a32dc967a1665d274cdc0e39d44c87bb
Log: Merge the fix for #61964 to 5.3, which will fix #63304
 [2014-10-07 23:35 UTC] stas@php.net
Automatic comment on behalf of reeze.xia@gmail.com
Revision: http://git.php.net/?p=php-src-security.git;a=commit;h=1d2f61904987133d542c68cd349cf313d0bef1c8
Log: Fixed bug #61964 (finfo_open with directory cause invalid free)
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Tue Dec 03 17:01:29 2024 UTC