Hi!
Recently bug 47358 came to my attention, and there's something strange
with it. It is fixed only for 5.6, not listed in NEWS for even that
version, but the log contains these:
commit f566b4d32183032663ecb35895c0dafd8e051853
Merge: 7a13ce6 10a2c0d
Author: Pierre Joye pierre.php@gmail.com
Date: Tue Jan 8 15:16:35 2013 +0100
Merge branch 'PHP-5.5'
* PHP-5.5:
- fix bug #47358, glob returns error, should be empty array()
- NEWS entry for #50524
commit 10a2c0d574be62be1f0224d23909e27e74c87445
Merge: f4142a9 2fb7cd3
Author: Pierre Joye pierre.php@gmail.com
Date: Tue Jan 8 15:13:40 2013 +0100
Merge branch 'PHP-5.4' into PHP-5.5
* PHP-5.4:
- fix bug #47358, glob returns error, should be empty array()
- NEWS entry for #50524
However, no trace of fixes for 47358 in either 5.4 or 5.5. Could
somebody explain what happened there? I couldn't find any discussion on
this bug except for this:
http://grokbase.com/t/php/php-internals/11a9mgeftd/bug-47358-unexpected-behaviour
Is there any reason why the fix is only in 5.6? Any reason why it's not
in the NEWS?
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
hi Stas, Jakub,
Hi!
Recently bug 47358 came to my attention, and there's something strange
with it. It is fixed only for 5.6, not listed in NEWS for even that
version, but the log contains these:commit f566b4d32183032663ecb35895c0dafd8e051853
Merge: 7a13ce6 10a2c0d
Author: Pierre Joye pierre.php@gmail.com
Date: Tue Jan 8 15:16:35 2013 +0100Merge branch 'PHP-5.5' * PHP-5.5: - fix bug #47358, glob returns error, should be empty array() - NEWS entry for #50524commit 10a2c0d574be62be1f0224d23909e27e74c87445
Merge: f4142a9 2fb7cd3
Author: Pierre Joye pierre.php@gmail.com
Date: Tue Jan 8 15:13:40 2013 +0100Merge branch 'PHP-5.4' into PHP-5.5 * PHP-5.4: - fix bug #47358, glob returns error, should be empty array() - NEWS entry for #50524However, no trace of fixes for 47358 in either 5.4 or 5.5. Could
somebody explain what happened there? I couldn't find any discussion on
this bug except for this:
http://grokbase.com/t/php/php-internals/11a9mgeftd/bug-47358-unexpected-behaviourIs there any reason why the fix is only in 5.6? Any reason why it's not
in the NEWS?
Sorry, I missed that one. I suspect it was the time where I
misconfigured git and branches did not get merged. Feel free to merge
it back/backport as I won't be able to do it before Monday.
Cheers,
Pierre
@pierrejoye | http://www.libgd.org
Hi!
Sorry, I missed that one. I suspect it was the time where I
misconfigured git and branches did not get merged. Feel free to merge
it back/backport as I won't be able to do it before Monday.
OK, I have backported it (and following Anatol's patches for it).
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
Hi,
On Fri, Aug 15, 2014 at 2:19 AM, Stas Malyshev smalyshev@sugarcrm.com
wrote:
Hi!
Sorry, I missed that one. I suspect it was the time where I
misconfigured git and branches did not get merged. Feel free to merge
it back/backport as I won't be able to do it before Monday.OK, I have backported it (and following Anatol's patches for it).
The Anatol patch (
https://github.com/php/php-src/commit/ad492ca9327fc9f7f0ea7a0ddd32e62cdf0c9137)
is actually wrong. I remember that we discussed in the PR 398. The
open_basedir cannot be changed from $path/... to /tmp (it's not tightening).
The source of the problem is that the open_basedir will always leek
information about dirs that the user is not suppose to see (out of the
open_basedir restriction). The reason is that false value can indicate that
(more info in the PR or https://bugs.php.net/bug.php?id=65489 ). The
solution would be to either return empty array instead (it could be
considered as a BC break) or re-implement glob. The re-implementaion could
be also interesting for perf (removing double allocations - glob + zval)
when used for dirs with many items but that would be quite an effort which
I'm not sure it's worth it... :).
However not sure if it's so big issue as no one should ever use
open_basedir in security context anyway... :)
Cheers
Jakub
Hi!
The source of the problem is that the open_basedir will always leek
information about dirs that the user is not suppose to see (out of the
open_basedir restriction). The reason is that false value can indicate
that (more info in the PR or https://bugs.php.net/bug.php?id=65489 ).
However not sure if it's so big issue as no one should ever use
open_basedir in security context anyway... :)
The biggest problem with glob() was not security but the fact that it
returned false on non-existing files when open_basedir is set, even when
everything is inside open_basedir context. Ideally, glob() should return
the same with and without open_basedir, with added restriction that if
open_basedir disallows access to some dir/file, then this dir/file does
not exist for the purposes of glob(). I'm not sure though if glob() does
that right now completely or how hard it would be to fix it, I didn't
look into it deeper. I just noticed that a very old bug was not fixed in
5.4/5.5 by mistake, and this bug makes usage of glob with open_basedir
very annoying, as you can not use glob() in foreach() anymore.
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
Hi,
On Fri, Aug 15, 2014 at 7:59 PM, Stas Malyshev smalyshev@sugarcrm.com
wrote:
Hi!
The source of the problem is that the open_basedir will always leek
information about dirs that the user is not suppose to see (out of the
open_basedir restriction). The reason is that false value can indicate
that (more info in the PR or https://bugs.php.net/bug.php?id=65489 ).
However not sure if it's so big issue as no one should ever use
open_basedir in security context anyway... :)The biggest problem with
glob()was not security but the fact that it
returned false on non-existing files when open_basedir is set, even when
everything is inside open_basedir context. Ideally,glob()should return
the same with and without open_basedir, with added restriction that if
open_basedir disallows access to some dir/file, then this dir/file does
not exist for the purposes ofglob(). I'm not sure though ifglob()does
that right now completely or how hard it would be to fix it, I didn't
look into it deeper. I just noticed that a very old bug was not fixed in
5.4/5.5 by mistake, and this bug makes usage of glob with open_basedir
very annoying, as you can not useglob()in foreach() anymore.
The Pierre's commit is fine. It fixes the problem that you just described.
So well done for backporting it... ;)
However the second commit from Anatol is unrelated to the Pierre's and
doesn't really fix anything. It was probably forgotten there when we all
forgot about that problem... :)
Anyway there are probably more important things than securing open_basedir
that is not secure anyway :)
Cheers
Jakub