php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #68665 Invalid free
Submitted: 2014-12-28 03:54 UTC Modified: 2015-01-06 00:41 UTC
From: honey at internot dot info Assigned: ab
Status: Closed Package: *General Issues
PHP Version: master-Git-2014-12-28 (Git) OS: Linux Ubuntu 14.04
Private report: No CVE-ID:
 [2014-12-28 03:54 UTC] honey at internot dot info
Description:
------------
Hi,

In /ext/fileinfo/libmagic/apprentice.c, an invalid free occurs:


1173                char mfn[MAXPATHLEN];
[..]
1198                                        efree(mfn);


as an array is not heap allocated, it cannot be freed.


Patches

php54_68665 (last revision 2016-12-12 03:01 UTC) by 12113362 at qq dot com)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2014-12-28 04:01 UTC] honey at internot dot info
And in /Zend/zend_language_scanner.l:


1934                s = Z_STRVAL_P(zendlval);
[..]
1938                efree(s);
 [2014-12-28 13:03 UTC] ab@php.net
Automatic comment on behalf of ab
Revision: http://git.php.net/?p=php-src.git;a=commit;h=a72cd07f2983dc43a6bb35209dc4687852e53c09
Log: Fixed bug #68665 (Invalid free)
 [2014-12-28 13:03 UTC] ab@php.net
-Status: Open +Status: Closed
 [2014-12-28 13:21 UTC] ab@php.net
-Assigned To: +Assigned To: ab
 [2014-12-28 13:21 UTC] ab@php.net
@honey, the first one is a good catch, affects also 5.6. But with the second one - can you provide a test case for that? 

Thanks
 [2014-12-28 15:39 UTC] ab@php.net
@honey, ok, this is fixed. Was reproducable only with big5 in mbstring. Thanks.
 [2014-12-28 17:45 UTC] honey at internot dot info
Did you make a test-case? If so, could you paste it here?

Thanks
 [2014-12-28 19:59 UTC] ab@php.net
Yeah, i made it http://git.php.net/?p=php-src.git;a=blob;t;h=74ff01da332d7e9cf419dbaadec6487c4a78e2ef . However it seems that run-tests.php isn't able to repro this. Namely, it needs the ini not to be passed on command line, but to load a true ini file. But that's another topic about the module loading order. Using the test code and an ini file the latest fix in master should be cleared.

Thanks.
 [2014-12-28 20:18 UTC] honey at internot dot info
Ok, cool.

Should I submit a CVE-ID request to MITRE/oss-security?


Thanks,
 [2014-12-28 20:47 UTC] ab@php.net
Hmm, I'd say no. The language scanner one is master only, so shouldn't have been used in any production.

Offtop - I were glad to push with your names as git doesn't accept pure emails for author names. Please post that next time )

Thanks.
 [2014-12-28 21:08 UTC] honey at internot dot info
Ok cool, my name is Joshua Rogers for reference. When I submitted my various bug reports, I don't think it had an option.

Thanks,
 [2014-12-28 21:14 UTC] honey at internot dot info
Also, I'll contact MITRE in private to see if a non-production, master-git-only falls within their scope, just in case.

Thanks again,
 [2014-12-29 19:31 UTC] honey at internot dot info
OK, so, 

apprentice.c has beeen assigned: CVE-2014-9425


this was MITRE's response regarding that zend_language_scanner.l one:

--


There is currently no CVE ID for this. The practice that we follow is
not the same for every piece of software. For example, in the past we
have assigned CVE IDs for vulnerabilities in FFmpeg that did not
affect any FFmpeg release. The rationale for this is that Google was
incorporating unreleased FFmpeg code into Chrome. In the case of PHP,
we do not know of (for example) current cases in which a Linux
distribution ships packages based on using the PHP master tree at an
arbitrary point in time. Also, we have not seen PHP maintainers
advertise that end users should individually use master. Accordingly,
for PHP, master seems to not directly correspond to a "product," and
at least some of the bugs are a reflection of the code being in an
indeterminate development state.

--



Thanks,
 [2014-12-30 06:10 UTC] honey at internot dot info
My bad. It was actually CVE-2014-9426...
 [2014-12-30 18:50 UTC] ab@php.net
Ok, see your messages on the OSS security lists :)
 [2014-12-31 22:06 UTC] tyrael@php.net
next time please instead of Bug Type:Bug make sure to use Bug Type: Security and/or drop a mail to security@php.net
Makes it much easier to track security issues for the Release Managers, makes it easier to have a proper fix and for high impact issues we usually prefer to wait for the next release before going public with the bug/fix.
 [2015-01-01 10:07 UTC] honey at internot dot info
Ok, will do(as I've done for 2 just now).

Also @ab, did you make a test-case for the apprentice.c one? A quick look shows it's probably not possible, since it only happens on a CAST() failure, which I don't think is possible to trigger.

Thanks,
 [2015-01-01 18:57 UTC] ab@php.net
@honey, yeah, not that easy to trigger that one as it'd only depend on erealloc() fail. So no tests for that one. And on the other hand, it wloud be only happening when an external magic is used (and that is dangerous by it self) which is not recommended to do.

Thanks.
 [2015-01-01 20:00 UTC] nikic@php.net
This can't happen at all, because erealloc() is infallible - it will cause a bailout (longjmp) if it can't perform the allocation.

We probably only keep these checks around to keep the libmagic.patch minimal.
 [2015-01-05 18:12 UTC] tyrael@php.net
so it seems that this wasn't really a bug.
do we wanna send a headsup to mitre to revoke/reclassify the issue?
I don't think that it is plausible that anybody would ever use an erealloc implementation which doesn't bail out on failure but return NULL, so I think that the current score for this is pretty misleading.
 [2015-01-06 00:41 UTC] honey at internot dot info
Sure thing, I'll send it through.

Thanks,
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Sat Apr 29 05:01:54 2017 UTC