Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check if ini_set is available to prevent Fatal Errors #1054

Open
maltfield opened this issue Oct 11, 2024 · 4 comments
Open

check if ini_set is available to prevent Fatal Errors #1054

maltfield opened this issue Oct 11, 2024 · 4 comments

Comments

@maltfield
Copy link
Contributor

maltfield commented Oct 11, 2024

There is a bug in phpList that causes Fatal Errors on PHP servers that have been hardened following common best-practices

    ini_set('session.name','phpListSession');

This line causes a PHP Fatal error on hardened systems with the ini_set function disabled.

PHP Fatal error:  Uncaught Error: Call to undefined function ini_set() in /path/to/public_html/lists/admin/init.php:17\\nStack trace:\\n#0 /path/to/public_html/lists/index.php(31): require_once()\\n#1 {main}\\n  thrown in /path/to/public_html/lists/admin/init.php on line 17

Why this matters

For security reasons, orgs frequently configure php.ini to be hardened by adding many dangerous functions to the disable_functions variable in the php.ini file. For example, it's common to disable the exec function

disable_functions = exec

Of course, if a php script could modify the php configuration, then it would defeat any hardening done by setting disable_functions. As such, it's common to add ini_set to the disable_functions

disable_functions = exec, ini_set

Solution

To fix the PHP Fatal error, phpList should always check to see if the ini_set function exists before attempting to call it

    if( function_exists( 'ini_set') ){
        ini_set('session.name','phpListSession');
        ini_set('session.cookie_samesite','Strict');
        ini_set('session.use_only_cookies',1);
        ini_set('session.cookie_httponly',1);
    }
@maltfield
Copy link
Contributor Author

maltfield commented Oct 11, 2024

Note that, while this has always triggered error messages, it's only since PHP 8 that this now triggers a Fatal Error.

@bramley
Copy link
Contributor

bramley commented Oct 12, 2024

Of course, if a php script could modify the php configuration, then it would defeat any hardening done by setting disable_functions

disable_functions cannot be changed by ini_set()

@michield
Copy link
Member

I think the point of @maltfield is that disable_functions is set outside of phpList somewhere.

Counter point would be to not disable ini_set for phpList for hardening, as it would defeat the hardening that phpList makes using it

@maltfield
Copy link
Contributor Author

Update: phpList v3.6.15 (the latest version at the time of writing) is now throwing fatal errors for two additional functions on servers with php security-hardened following best-practices:

  1. parse_ini_file
  2. putenv

Here's example errors messages indicating where the fixes need to take place (see OP for the solution wrapping these function calls with a conditional check of function_exists()):

PHP Fatal error:  Uncaught Error: Call to undefined function parse_ini_file() in /path/to/public_html/lists/admin/languages.php:19\nStack trace:\n#0 /path/to/public_html/lists/admin/index.php(103): include_once()\n#1 {main}\n  thrown in /path/to/public_html/lists/admin/languages.php on line 19'

and

PHP Fatal error:  Uncaught Error: Call to undefined function putenv() in /path/to/public_html/lists/admin/languages.php:244\nStack trace:\n#0 /path/to/public_html/lists/admin/languages.php(507): phplist_I18N->gettext()\n#1 /path/to/public_html/lists/admin/languages.php(529): phplist_I18N->getTranslation()\n#2 /path/to/public_html/lists/admin/languages.php(571): phplist_I18N->get()\n#3 /path/to/public_html/lists/admin/defaultconfig.php(106): s()\n#4 /path/to/public_html/lists/admin/index.php(104): require_once('...')\n#5 {main}\n  thrown in /path/to/public_html/lists/admin/languages.php on line 244'

Please let me know if I should open separate tickets for each of these functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants