Fixed bug #61964 (finfo_open with directory cause invalid free) #91

Merged
merged 1 commit into from Jul 15, 2012

4 participants

@reeze

Hi,
we have been discuss this issue in irc, @laruence got some concern,

then I sent a mail to the file lib author: Christos Zoulas christos@zoulas.com.
he replied:

I got two question to ask:
1. is file intend to support dir?
2. does the double free is bug?
Yes, yes, and I don't know what version you are using but the current version
does asprintf() not snprintf().

both 5.3&5.4 didn't use asprintf(), we use our memory management functions.

after got his reply I think I can send this PR to ask you guys comment.

Thanks.

@laruence
php.net member

..... Double free is no doubt a bug, the reason I care is lib magic is not ready for that in php, as I said there even was code fprintf(stderr... So I think it should be whole examed by somebody who really know it. But not do a little fix one by one,

Thanks

@reeze

Hm,.. from the commit log , seems Felipe & Pierre knows about it, we could ask them to improve it.

@nikic

Thanks for looking into this @reeze :)

@laruence
php.net member

for the record, as felipe said, this patch cause segfault. need to be re-examed.
it's really bad we having two conversiones both at here and bugs.php.net...

@php-pulls

Comment on behalf of stas at php.net:

Looks like the patch is not right (see the bug, it segfaults) so please fix and resubmit the pull, I'll close this one for now.

@reeze

Hi, stas:
The segfault mentioned by felipe have been fixed right after felipe reported in the bug, and I've added the test case:

https://github.com/php/php-src/pull/91/files#L1R31.

Thanks for reply!

@reeze

BTW: The reason there is only on commit is that I want to make the commit clean I update the PR by force-update.

@php-pulls php-pulls merged commit 1d2f619 into php:master Jul 15, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment