Edit report at https://bugs.php.net/bug.php?id=71220&edit=1
ID: 71220
Updated by: yohgaki@php.net
Reported by: hugh at allthethings dot co dot nz
Summary: Null pointer deref (segfault) in compact via
ob_start
Status: Closed
Type: Bug
Package: Reproducible crash
Operating System: Linux
PHP Version: 7.0.1
Assigned To: laruence
Block user comment: N
Private report: N
New Comment:
It's better not to crash by broken code.
However, one may crash PHP very easily by broken code
<?php
function foo() {
foo();
}
foo();
?>
crashes. We should protect PHP from internal memory manipulation/leak, but I'm not sure if
crash protections for broken code worth the effort/additional overhead.
Previous Comments:
------------------------------------------------------------------------
[2016-01-11 22:00:51] yohgaki@php.net
bin2hex() wouldn't work due to signature because it's not designed for ob_start().
However, the argument itself is valid.
ob_start() must accept internal functions as callback. Examples are ob_gzhandler(), etc. http://php.net/ob_gzhandler
------------------------------------------------------------------------
[2016-01-11 20:53:18] hugh at allthethings dot co dot nz
Hi Nikic,
Thanks for that detailed reply!
You mention in your first paragraph that calling bin2hex via ob_start won't work due to
mismatched argument counts, though in bug #70290 I found that the Zend engine gets arguments through
its own way even if they don't exist. In that case it was calling spl_autoload, which expected
1 or 2 arguments, but ob_start passed it both the buffer and the phase, where the second argument
ob_start is meant to pass is an int, but spl_autoload treated it as a string.
I think you are hitting the nail full on by noticing the mismatch in the lower level stack
reading/changing functions when used as callbacks. And I agree with the need to ban them. Prehaps a
blacklist of functions that make no sense, or a whitelist of core defined functions that do make
sense (and of course any user defined functions).
I would be happy to work on a patch for that if it would be likely to be accepted, with direction of
what core devs would like. If that is the case, should I create a new bug for that?
Cheers,
Hugh
------------------------------------------------------------------------
[2016-01-11 20:32:52] nikic@php.net
@hugh: The test file has been fixed in a commit shortly after that.
As to the root cause of this issue: There is nothing inherently wrong with using an internal
function as the callback to ob_start(). Just think about something like
ob_start('bin2hex') to create a hex output stream. (It will not actually work due to
argument count mismatch, but you get the idea.)
The problem that I see here is that we allow functions those purpose is to expect userland stack
frames to be called dynamically. This not only causes the segfaults in this and related bug reports,
but the behavior of these functions as callbacks is generally ill-defined.
For example, consider this piece of code:
namespace Foo {
function test($a, $b, $c) {
var_dump(call_user_func('func_get_args'));
var_dump(call_user_func('get_defined_vars'));
}
test(1, 2, 3);
}
namespace {
function test($a, $b, $c) {
var_dump(call_user_func('func_get_args'));
var_dump(call_user_func('get_defined_vars'));
}
test(1, 2, 3);
}
The output under PHP 7 is:
array(1) {
[0]=> string(13) "func_get_args"
}
array(3) {
["a"]=> int(1)
["b"]=> int(2)
["c"]=> int(3)
}
array(3) {
[0]=> int(1)
[1]=> int(2)
[2]=> int(3)
}
array(3) {
["a"]=> int(1)
["b"]=> int(2)
["c"]=> int(3)
}
So, in the former case, func_get_args() will inspect the parent call frame (whether it's an
internal function or not), so it returns the arguments passed to call_user_func (which is just
"func_get_args"). On the other hand, get_defined_vars() will inspect the next higher
userland stack frame, so it will instead act on function test().
In the latter case, func_get_args() now works on the user function again -- because here the
call_user_func() has been optimized away, so the next higher frame is the test() one.
On HHVM the first (namespaced) get_defined_vars() call instead produces this output:
array(2) {
["callback"]=> string(16) "get_defined_vars"
["parameters"]=> array(0) { }
}
So HHVM chooses to always use the next-higher stack frame here, even if it's an internal one.
In this case only the arguments of the function are used as variables.
Even odder, if you try do run
var_dump(array_map('extract', [['res' => 123]]));
under HHVM, you'll get an "Cannot use a scalar value as an array" warning, as this
particular variant of the array_map() function has been implemented using HHAS. In PHP 7, instead a
$res variable is created in the parent scope.
These stack-inspecting/manipulating functions really are more language items than functions, using
them as callbacks makes no sense, is ill-defined and causes confusing behavior. We should ban it.
------------------------------------------------------------------------
[2016-01-11 00:01:44] hugh at allthethings dot co dot nz
Hi,
Just looking at the patch for this [1], I notice that the test case has the extract function, not
the compact function. Just tested, the extract function doesn't have this issue, so as it is
the test isn't useful.
stas, did my comment above help you understand what I'm seeing as the root cause of this bug?
Cheers,
Hugh
[1] http://git.php.net/?p=php-src.git;a=commitdiff;h=c56efb848b01fa3ecdb7f7253b541b020d154290;hp=6700be67f58611d08bbacc44f327ce98ed0473c9
------------------------------------------------------------------------
[2015-12-27 23:26:04] hugh at allthethings dot co dot nz
Hi stas,
I mean functions that are defined in the Zend c language space rather than php userland which I
assume is what ob_start is intended for. Here the compact is. Actually calling zif_compact and in
the other vug reports they are also zif_ functions. Sorry about confusion about what I meant with
Zend defined. Hopefully this clears it uo
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=71220
--
Edit this bug report at https://bugs.php.net/bug.php?id=71220&edit=1