Add configuration value to enable/disable stack trace logging

[eriklundin]

Background:
The latest version of PHP seems to handle fatal errors as exceptions which results in stack traces being logged. Stack traces can potentially contain sensitive information and should not be logged in a production environment.

Test code:

<?php
function handle_password($a) {
        does_not_exist();
}
handle_password('s3cretp4ssword');

PHP 5.4.16:
Jun 17 15:58:01 server php[29650]: PHP Fatal error: Call to undefined function does_not_exist() in /var/www/html/index.php on line 3

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#012Stack trace:#12#0 /var/www/html/index.php(5): handle_password('s3cretp4ssword')#12#1 {main}#12 thrown in /var/www/html/index.php on line 3

Suggested patch:
Add a configuration value to be able to prevent exceptions from logging stack traces.

log_exception_trace = On/Off

It would be optimal to have this disabled as default as novice administrators would perhaps not be aware that this information would be logged. For debugging purposes it would be helpful to be able to enable this but maybe the default value should be set conservatively to minimize unnecessary problems?

I've added this configuration value in Zend/zend.c as the exception message is compiled in Zend/zend_exceptions.c. Adding it to main/main.c would change the scope from zend_compiler_globals to php_core_globals and I guess that you wouldn't want to mix them?

[nikic]

I think my general preference here would be not to influence whether the trace is logged, but whether backtrace arguments are collected in the first place. It may be desirable to disable their collection for other reasons as well (see https://bugs.php.net/bug.php?id=75056, though I'm sure we have a few other bug reports on this as well).

[eriklundin]

I agree that the stack trace never should be collected for logging there in the first place. Potentially writing sensitive data like passwords in clear text for whatever reason is pretty serious. This patch would be minimal impact as it would be possible to enable the old behaviour with minimal effort. I don't know if people rely on parsing the old output?

[krakjoe]

Without a stack trace exceptions are quite useless, what Nikita is suggesting is that this setting should only effect arguments being collected (and so displayed) in traces. Exceptions need traces whatever, a setting that disables them makes little sense.

…

On Mon, 17 Jun 2019, 19:20 eriklundin, ***@***.***> wrote: I agree that the stack trace never should be collected for logging there in the first place. Potentially writing sensitive data like passwords in clear text for whatever reason is pretty serious. This patch would be minimal impact as it would be possible to enable the old behaviour with minimal effort. I don't know if people rely on parsing the old output? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#4281?email_source=notifications&email_token=AARB52WJO7DSGVPAMXTVJA3P27BXDA5CNFSM4HYYRRX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODX33VFQ#issuecomment-502774422>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AARB52TWZHXU7S23LVBN423P27BXDANCNFSM4HYYRRXQ> .

[eriklundin]

I wouldn't agree that exceptions without stack traces are useless. You still get the line number and file in which it occurred. It would of course be very helpful for a developer to get all the facts needed from log files but the bare minimum would be file and line number. If your program has it's own error handler you could choose to log the stack trace or not but this is the default behaviour of unhandled fatal errors. As of now, it logs arguments in clear text which i would say is a serious flaw regardless of the optimal long term solution.

The other option then. Obfuscating the data. Would storing/printing three question marks "???" instead of the actual data (when it's turned off) be sufficient? Would it also prevent manually getting and printing this data (ex: debug_backtrace())

[dstogov]

I'm agree with @nikic
We should prevent collecting sensitive data, not just logging them.
In case we had attributes, we might use them to disable collection...

[nikic]

I think the consensus is to go with #4282 instead, so I'm closing this one.