php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Request #68827 Double free with disabled ZMM
Submitted: 2015-01-13 18:04 UTC Modified: 2015-01-22 08:56 UTC
From: bugreports at internot dot info Assigned: ab (profile)
Status: Closed Package: *General Issues
PHP Version: master-Git-2015-01-13 (Git) OS: Linux Ubuntu 14.04
Private report: No CVE-ID: None
View Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
If you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: bugreports at internot dot info
New email:
PHP Version: OS:

 

 [2015-01-13 18:04 UTC] bugreports at internot dot info
Description:
------------
Hi,

In /ext/fileinfo/libmagic/apprentice.c:

2609        if ((map = CAST(struct magic_map *, ecalloc(1, sizeof(*map)))) == NULL) {
2610                file_oomem(ms, sizeof(*map));
2611                efree(map);
2612                goto error;
2613        }

That goes to error:

2730error:
2731        if (stream) {
2732                php_stream_close(stream);
2733        }
2734        apprentice_unmap(map);

which as you can see, does a double free of 'map'.

The line in the apprentice_unmap function:
499        if (map == NULL)
is kind of useless, because even if it has already been freed, it won't be NULL(unless the php implementation of efree does something different?)


Thanks,


Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-01-14 09:35 UTC] jpauli@php.net
A map=NULL seems to be missing.
efree() does nothing more than free().
 [2015-01-14 09:42 UTC] tony2001@php.net
-Status: Open +Status: Feedback
 [2015-01-14 09:42 UTC] tony2001@php.net
The lines in if (map == ..) won't be executed ever when using Zend memory manager, because it bails out immediately on OOM error, so the only way to get them executed is to disable Zend MM and go with system MM.

man free says: 
If ptr is NULL, no operation is performed.

apprentice_unmap():
    if (map == NULL)
        return;

So.. where is the problem here?
 [2015-01-14 12:17 UTC] bugreports at internot dot info
-Status: Feedback +Status: Open
 [2015-01-14 12:17 UTC] bugreports at internot dot info
'map' is not NULL. efree(map) doesn't set map=NULL;.
 [2015-01-14 12:33 UTC] tony2001@php.net
-Status: Open +Status: Feedback
 [2015-01-14 12:33 UTC] tony2001@php.net
That's correct, efree() doesn't modify the pointer.
But it's already NULL at the time efree() is called, take a look at the if condition:

    if ((map = CAST(struct magic_map *, ecalloc(1, sizeof(*map)))) == NULL) {
        file_oomem(ms, sizeof(*map));
        efree(map);
        goto error;
    }
 [2015-01-14 12:44 UTC] bugreports at internot dot info
-Status: Feedback +Status: Open
 [2015-01-14 12:44 UTC] bugreports at internot dot info
Ahh, I see!

Well, then, this code:

2611                efree(map);
2612                goto error;

should just be replaced with 
return NULL;
since 'dbname', and 'stream' are not used at that stage.

Thanks,
 [2015-01-14 12:53 UTC] tony2001@php.net
-Type: Security +Type: Feature/Change Request
 [2015-01-14 23:19 UTC] stas@php.net
-Status: Open +Status: Not a bug
 [2015-01-14 23:19 UTC] stas@php.net
This is a file from fileinfo library, so it may make sense to report the issue here; http://bugs.gw.com/my_view_page.php

In this particular case return NULL may indeed be faster, but I don't see any security issue or PHP-related bug here.
 [2015-01-22 00:31 UTC] pajoye@php.net
-Status: Not a bug +Status: Open
 [2015-01-22 00:31 UTC] pajoye@php.net
@stas 

as it is a fileinfo issue and should also reported there, we do bundle it, patched. So it affects PHP more directly too. We should apply the fix.
 [2015-01-22 07:44 UTC] ab@php.net
-Assigned To: +Assigned To: ab
 [2015-01-22 07:44 UTC] ab@php.net
Looks like it's an issue in the PHP patch, the latest libmagic looks fine https://github.com/file/file/blob/master/src/apprentice.c#L1283 . Despite it's only an issue with disabled ZMM, worth fixing.

Thanks.
 [2015-01-22 08:56 UTC] ab@php.net
-Summary: Double free +Summary: Double free with disabled ZMM
 [2015-01-22 09:04 UTC] ab@php.net
Automatic comment on behalf of bugreports@internot.info
Revision: http://git.php.net/?p=php-src.git;a=commit;h=91aa340180eccfc15d4a143b54d47b8120f898be
Log: Fixed bug #68827 Double free with disabled ZMM
 [2015-01-22 09:04 UTC] ab@php.net
-Status: Assigned +Status: Closed
 [2015-01-22 09:05 UTC] ab@php.net
Automatic comment on behalf of bugreports@internot.info
Revision: http://git.php.net/?p=php-src.git;a=commit;h=91aa340180eccfc15d4a143b54d47b8120f898be
Log: Fixed bug #68827 Double free with disabled ZMM
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Wed Oct 09 07:01:28 2024 UTC