Bug #71340 [Com]: php_admin_value[error_reporting] in fpm/apache conf can be bypassed in user code

From: Date: Sun, 06 May 2018 14:34:36 +0000
Subject: Bug #71340 [Com]: php_admin_value[error_reporting] in fpm/apache conf can be bypassed in user code
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-215124@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=71340&edit=1

 ID:                 71340
 Comment by:         gpointorama at gmail dot com
 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:

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 :|


Previous Comments:
------------------------------------------------------------------------
[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);

------------------------------------------------------------------------
[2018-05-06 13:41:49] requinix@php.net

Hard to say exactly what changed between 5 and 7 to be responsible for this, but I think the change
to bring back this behavior is straightforward: in error_reporting(), only do the change if
p->modifiable allows for it. Or possibly switch to using zend_alter_ini_entry() instead.

------------------------------------------------------------------------
[2018-05-06 13:27:41] admin at inwebse dot com

Will be there any answer from php kernel developers?

------------------------------------------------------------------------
[2018-05-02 19:26:46] spam2 at rhsoft dot net

besides that fpm seems to have still a lot of issues given that it is called the recommended way to
run PHP versus a rock-stable mod_php:

"a composer module sets error_reporting(E_ALL) which breaks parts of our app due to E_NOTICE
being thrown" is no compliement for your app - in two aspects - a) it should run clean with
E_ALL and b) you must not spit out errors/warnings to the client

------------------------------------------------------------------------


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


Thread (28 messages)

« previous php.bugs (#215124) next »