Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:105968
Content-Type: multipart/alternative;
	boundary=Apple-Mail-F3CAE952-40B2-4DEE-A9D2-31CA281638D1
Mime-Version: 1.0 (1.0)
In-Reply-To: <d59417b0-f08d-fc03-6eaf-665abd6a5f18@telia.com>
Date: Mon, 17 Jun 2019 21:38:55 +0200
Cc: PHP internals <internals@lists.php.net>
Content-Transfer-Encoding: 7bit
Message-ID: <6DE35C14-DC1A-49C2-BCCB-58EEDB548EF8@coretech.se>
References: <alpine.LRH.2.21.1906171908350.27190@supernova.coretech.se> <d59417b0-f08d-fc03-6eaf-665abd6a5f18@telia.com>
To: =?utf-8?Q?Bj=C3=B6rn_Larsson?= <bjorn.x.larsson@telia.com>
Subject: Re: [PHP-DEV] [PATCH] Add configuration value to enable/disable stack trace logging
From: erik@coretech.se (Erik Lundin)

--Apple-Mail-F3CAE952-40B2-4DEE-A9D2-31CA281638D1
Content-Type: text/plain;
	charset=utf-8
Content-Transfer-Encoding: quoted-printable


Joe=E2=80=99s solution seems to fix the problem. I havent tested it yet thou=
gh. I would have been forced to patch this reguardless before bringing php 7=
+ into production. His fix would be enough to protect the data provided prop=
er config files are enforced.

Thanks Joe! Hopefully this will be merged which would be one less thing to m=
aintain.

/Erik

> 17 juni 2019 kl. 21:27 skrev Bj=C3=B6rn Larsson <bjorn.x.larsson@telia.com=
>:
>=20
>> Den 2019-06-17 kl. 19:10, skrev Erik Lundin:
>> Background:
>> The latest version of PHP seems to handle fatal errors as exceptions whic=
h results in stack traces being logged. Stack traces can potentially contain=
 sensitive information and should not be logged in a production environment.=

>>=20
>> Test code:
>> <?php
>> function handle_password($a) {
>>         does_not_exist();
>> }
>> handle_password('s3cretp4ssword');
>>=20
>> PHP 5.4.16:
>> Jun 17 15:58:01 server php[29650]: PHP Fatal error:  Call to undefined fu=
nction does_not_exist() in /var/www/html/index.php on line 3
>>=20
>> PHP 7.4 (dev):
>> Jun 17 15:58:01 server php[18159]: PHP Fatal error:  Uncaught Error: Call=
 to undefined function does_not_exist() in /var/www/html/index.php:3#012Stac=
k trace:#012#0 /var/www/html/index.php(5): handle_password('s3cretp4ssword')=
#012#1 {main}#012  thrown in /var/www/html/index.php on line 3
>>=20
>> Suggested patch:
>> Add a configuration value to be able to prevent exceptions from logging s=
tack traces.
>>=20
>> log_exception_trace =3D On/Off
>>=20
>> It would be optimal to have this disabled as default as novice administra=
tors would perhaps not be aware that this information would be logged. For d=
ebugging purposes it would be helpful to be able to enable this but maybe th=
e default value should be set conservatively to minimize unnecessary problem=
s?
>>=20
>> I've added this configuration value in Zend/zend.c as the exception messa=
ge is compiled in Zend/zend_exceptions.c. Adding it to main/main.c would cha=
nge the scope from zend_compiler_globals to php_core_globals and I guess tha=
t you wouldn't want to mix them?
>>=20
>> Link to pull request: https://github.com/php/php-src/pull/4281
>>=20
>> Regards, Erik Lundin
>>=20
> Hi,
>=20
> In our environment these kind of errors goes to the Apache error log.
> We have full control of the complete LAMP stack and it's only our
> ISP who can access these. So little risk for leakage of sensitive info.
>=20
> Now we have a large legacy code base that has been migrated from
> PHP 5.2 to PHP 7.3. Even if we have tested a lot we have had great
> usage of these kind of stack traces in the production environment.
> It helped us fixing the (hopefully) last issues. We also have a kind of
> beta site, but we didn't got enough traffic on that one to trigger the
> errors we got in production.
>=20
> My 50c on this subject, well aware of that having "ownership" of the
> LAMP stack is one prerequisite to not disclose sensitive info.
>=20
> r//Bj=C3=B6rn Larsson

--Apple-Mail-F3CAE952-40B2-4DEE-A9D2-31CA281638D1--