Edit report at https://bugs.php.net/bug.php?id=71340&edit=1
ID: 71340
Updated by: requinix@php.net
Reported by: gpointorama at gmail dot com
Summary: php_admin_value[error_reporting] in fpm/apache conf
can be bypassed in user code
Status: Analyzed
Type: Bug
Package: PHP options/info functions
Operating System: Any
PHP Version: 7.0
Block user comment: N
Private report: N
New Comment:
The thing is, error_reporting is more than just an INI value. It has a much larger impact on PHP at
runtime than options like include_path do. And while include_path probably only needs to be set once
or twice in code, during an initialization phase, it's perfectly reasonable to change the
error_reporting value at any time in any number of circumstances.
If no changes to the error reporting level are allowed at all, what about the @ operator? Should
that be blocked because it temporarily suppresses the level entirely?
Perhaps a php_admin_value for error_reporting should be treated as more than just an immutable
number. Say, as a bitmask that is always ORed with a desired new value? That would guarantee errors
of a certain type are always visible while allowing code and libraries to opt into other types. I
think that would be a more common use case than needing to ensure a maximum level (ANDing) so code
would only be able to turn off error types - which is probably needed for specific situations that
could be addressed with careful use of @.
Previous Comments:
------------------------------------------------------------------------
[2018-05-06 14:55:12] spam2 at rhsoft dot net
there is nothing questionable - php_value versus php_admin_value and the same for 'flag'
has a defined behavior no matter what value you want not get changed by a script, on my servers I
would use it to disallow idiots lower the reporting level to force them write clean code running
with E_ALL or go away
------------------------------------------------------------------------
[2018-05-06 14:43:14] gpointorama at gmail dot com
Please also note that using php_admin_value has only one meaning, preventing change to ini directive
at user level, and also using php_admin_value is not mandatory, so i really can't see the
questionable part of the discussion.
Sorry for being raw, but i'm in a bad mood, and i can't find better words, despite also
not knowing english very well
------------------------------------------------------------------------
[2018-05-06 14:34:35] gpointorama at gmail dot com
ok this is a huge problem/change that complicate hosting of php sites a lot,
i can explain the details but i've already tried and no one cared,
at least list it on the backward incompatible list of things between php 5 and 7
AND in the doc, because the doc still explain the behavior of php5 :|
------------------------------------------------------------------------
[2018-05-06 14:27:04] nikic@php.net
error_reporting() was likely switched away from zend_alter_ini_entry() for performance reasons. In
any case changing this would be easy enough, but like whoever you talked to on IRC I am not
convinced that this is a bug. Preventing a change of error_reporting seems like a very questionable
practice to me, and I'm not sure we want to support this.
------------------------------------------------------------------------
[2018-05-06 14:19:07] gpointorama at gmail dot com
this is the required patch, ive talked about it on irc with php devs a while ago,
a test for this should also be added to be sure does not happens again,
it's obviusly a bug, someone at some point copy pasted the some wrong code from the utility
functions instead of calling it,
also there is some ppl that think the new backward incompatible and buggy behavior is right,
it's funny
please someone show some love to this bug :(
@@ -673,38 +673,10 @@ ZEND_FUNCTION(error_reporting)
old_error_reporting = EG(error_reporting);
if (ZEND_NUM_ARGS() != 0) {
zend_string *new_val = zval_get_string(err);
- do {
- zend_ini_entry *p = EG(error_reporting_ini_entry);
-
- if (!p) {
- p = zend_hash_find_ptr(EG(ini_directives), ZSTR_KNOWN(ZEND_STR_ERROR_REPORTING));
- if (p) {
- EG(error_reporting_ini_entry) = p;
- } else {
- break;
- }
- }
- if (!p->modified) {
- if (!EG(modified_ini_directives)) {
- ALLOC_HASHTABLE(EG(modified_ini_directives));
- zend_hash_init(EG(modified_ini_directives), 8, NULL, NULL, 0);
- }
- if (EXPECTED(zend_hash_add_ptr(EG(modified_ini_directives),
ZSTR_KNOWN(ZEND_STR_ERROR_REPORTING), p) != NULL)) {
- p->orig_value = p->value;
- p->orig_modifiable = p->modifiable;
- p->modified = 1;
- }
- } else if (p->orig_value != p->value) {
- zend_string_release(p->value);
- }
-
- p->value = new_val;
- if (Z_TYPE_P(err) == IS_LONG) {
- EG(error_reporting) = Z_LVAL_P(err);
- } else {
- EG(error_reporting) = atoi(ZSTR_VAL(p->value));
- }
- } while (0);
+ zend_string *ini_name;
+ ini_name = zend_string_init("error_reporting", sizeof("error_reporting") - 1,
0);
+ zend_alter_ini_entry(ini_name, new_val, ZEND_INI_USER, ZEND_INI_STAGE_RUNTIME);
+ zend_string_release(ini_name);
}
RETVAL_LONG(old_error_reporting);
------------------------------------------------------------------------
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=71340
--
Edit this bug report at https://bugs.php.net/bug.php?id=71340&edit=1