Why doesn't phplist use error_log()

Tags: #<Tag:0x00007f00e177a630> #<Tag:0x00007f00e177a518>


Can a phplist developer please tell me why phplist doesn’t use php’s built-in error_log() function?

For example, phplist’s file defines a custom logging function sqllog(). This sqllog() function takes a required argument of the log file path, which is hard-coded throughout the same file to be some files in “/tmp/”

^ This is extremely concerning, since “/tmp/” is often setup as a 777 directory (read/write permissions granted to all users, groups, and other). Therefore, if an sql query included sensitive content (ie: passwords, user pii, etc), that query may be readable to all users and processes on the server.

Is there something I’m missing as to why phplist intentionally avoids the error_log() function built-into php?

Hey @maltfield, thanks for the feedback.
I don’t think using error_log with the default configuration would be the best option as it would spam the error log. Instead, I am thinking about creating a new temporary folder where the access is limited to phpList and log the logs there, and also a user can configure the log file path in their config file.
Would this work for you?

1 Like

I’m definitely not a php expert, but I strongly lean towards using built-in functionality rather than reinventing the wheel.

I don’t think using error_log with the default configuration would be the best option as it would spam the error log.

But that’s why php.ini allows the admin to configure what is written to the error logs. For example, you could have the sql stats be written as a E_STRICT using trigger_error():

The default php.ini config does not log E_NOTICE or E_STRICT (and post 5.3, it will omit E_DEPRECATED as well):

; Error handling and logging ;

; By default, PHP is set to take action on all errors, notices and warnings EXCEPT
; those related to E_NOTICE and E_STRICT, which together cover best practices and
; recommended coding standards in PHP. For performance reasons, this is the
; recommend error reporting setting. Your production server shouldn’t be wasting
; resources complaining about best practices and coding standards. That’s what
; development servers and development settings are for.

; Error Level Constants:
; E_ALL - All errors and warnings (includes E_STRICT as of PHP 5.4.0)
; E_ERROR - fatal run-time errors
; E_RECOVERABLE_ERROR - almost fatal run-time errors
; E_WARNING - run-time warnings (non-fatal errors)
; E_PARSE - compile-time parse errors
; E_NOTICE - run-time notices (these are warnings which often result
; from a bug in your code, but it’s possible that it was
; intentional (e.g., using an uninitialized variable and
; relying on the fact it’s automatically initialized to an
; empty string)
; E_STRICT - run-time notices, enable to have PHP suggest changes
; to your code which will ensure the best interoperability
; and forward compatibility of your code
; E_CORE_ERROR - fatal errors that occur during PHP’s initial startup
; E_CORE_WARNING - warnings (non-fatal errors) that occur during PHP’s
; initial startup
; E_COMPILE_ERROR - fatal compile-time errors
; E_COMPILE_WARNING - compile-time warnings (non-fatal errors)
; E_USER_ERROR - user-generated error message
; E_USER_WARNING - user-generated warning message
; E_USER_NOTICE - user-generated notice message
; E_DEPRECATED - warn about code that will not work in future versions
; of PHP
; E_USER_DEPRECATED - user-generated deprecation warnings

; Common Values:
; E_ALL (Show all errors, warnings and notices including coding standards.)
; E_ALL & ~E_NOTICE (Show all errors, except for notices)
; E_ALL & ~E_NOTICE & ~E_STRICT (Show all errors, except for notices and coding standards warnings.)
; Default Value: E_ALL & ~E_NOTICE & ~E_STRICT & ~E_DEPRECATED
; Development Value: E_ALL
; Production Value: E_ALL & ~E_DEPRECATED & ~E_STRICT
error_reporting = E_ALL & ~E_DEPRECATED & ~E_STRICT

And, in any case, you can overwrite what error types are actually logged at runtime with error_reporting(). This is something that phplist already does

Bear in mind that hardened php configs limit which directories php can execute in with open_basedir. If you proceed, be sure to use sys_get_temp_dir()

But even then, the best you can do is make the dir/files in your new temp dir/log file 0600 (not sure if even that’s achievable) and owned by the web server (so still accessible by other vhosts). But if you use trigger_error(), then they’re passed to the web server’s logs which can actually be setup so they’re only readable by root…

The sqllog() function is used only for development testing:

if (isset($GLOBALS["developer_email"]) && !empty($GLOBALS["developer_logqueries"])) {

I guess that ‘/tmp’ was just a convenient place to put the log file for a single-user development machine.

If you are trying to use that function within a running phplist, not for developer testing, then I think that is outside of its original intention.