PHP Warning: ini_get_all() has been disabled for security reasons

[dd32]

As reported in the Support Forums, WordPress 4.6 calls ini_get_all() which may be disabled on some hosts (possibly due to an old PHP 5.3.x security vulnerability).

​https://wordpress.org/support/topic/warning-ini_get_all-has-been-disabled-for-security-reasons

This can be duplicated by adding disable_functions = ini_get_all to your php.ini file.

Looking at our usage of the ini_* functions in core, we:

• sometimes silence warnings from ini_set() - usually early in the bootstrap, so I'm assuming from before wp_debug_mode() is called.
• never silence ini_get() calls.

The current breakage on the affected hosts would be:

• A PHP Warning will be displayed on sites post-upgrade to 4.6.
• wp_raise_memory_limit() will fail to increase the memory limit, which may result in some admin pages not loading, or image uploads failing to create resized images.
• WSOD - Single-site installs will not increase their memory limit to 40M, multisite will fail to increase to 64M, both of these could cause WSOD if the memory limit was set arbitrarily low and the site was running plugins which use a lot of memory.

We should fix this to at least not present with a PHP Warning, although I recognise that almost any function could be in the disable_functions setting, some (such as phpinfo() and ini_*) are historically much more likely to be there.

See #32075 for introduction.

[dd32]

I primarily see two main options:

• Silence the PHP Warning 37680.diff​
• Check the disable_functions list (as we do elsewhere for phpinfo()) 37680.2.diff​

That then leaves the question of what to do when disabled, Do we return true anyway as a "We can't determine if it's changeable, so we're just assuming it probably is."? or do we just continue to return false as 4.6 currently does?

I've taken the angle that we should return true in the event we can't be sure, this may result in PHP warnings being emitted from ini_set(), however we already silence warnings on the ini_set()'s at least for the memory_limit (which is the only thing this function is used for currently). It's the is_array() block at the end of the function in both patches that's responsible for this behaviour.

[blobaugh]

From ​http://php.net/manual/en/function.ini-set.php

Returns the old value on success, FALSE on failure.

I think we should follow in line here an return a false value. That will signify a possible issue with the call. Alternatively it could return a WP_Error if having the ini_* functions available is a hard requirement.

[SergeyBiryukov]

#37695 was marked as a duplicate.

[dd32]

Replying to blobaugh:

I think we should follow in line here an return a false value. That will signify a possible issue with the call. Alternatively it could return a WP_Error if having the ini_* functions available is a hard requirement.

The problem is that ini_set() and ini_get() are most likely available, it's just that ini_get_all() is disabled which causes an inability to determine if the ini_set() will pass or not.

WP_Error (which went unchecked) or null could be also used thanks to core using strict type checking on the call (for some reason) - (true: changeable, false: not changeable, null or WP_Error: who knows). We'd still only want to bail in the event it was false and not null or WP_Error though.

[swissspidy]

wp_is_ini_value_changeable() is being called in wp_initial_constants(), where the ini_get() call is already silenced. Silencing the ini_get_all() call as well seems fine in that case.

[jeremyfelt]

Replying to dd32:

The problem is that ini_set() and ini_get() are most likely available, it's just that ini_get_all() is disabled which causes an inability to determine if the ini_set() will pass or not.

This seems like a good reason to silence it.

[BinaryKitten]

It's considered bad practice to silence calls as it doesn't actually do much other than not show to the world when it generates an error message. The error log will still be populating with the message on every load and additionally the silence operator actually slows the process down (even though it's minute).

The check version is better as it actually detects the problem and processes it

[swissspidy]

@dd32 Assigning to you for final review & commit.

We could then keep it in trunk for a bit for folks to test.

[jorbin]

route number 2: 37680.2.diff seems best to me.

[dd32]

In 38431:

Bootstrap: Check that ini_get_all() exists before calling it, allows us to work around hosts who disable the function for "security purposes".

Fixes #37680 for trunk.

[dd32]

re-open for 4.6.1

I took a slightly different route, instead of checking if the function was disabled, simply called function_exists() which in retrospect is the obvious method..

[jdgrimes]

@dd32 function_exists() ​doesn't detect disabled functions:

Note:
A function name may exist even if the function itself is unusable due to configuration or compiling options (with the image functions being an example).

[jeremyfelt]

Replying to jdgrimes:

@dd32 function_exists() ​doesn't detect disabled functions:

Note:
A function name may exist even if the function itself is unusable due to configuration or compiling options (with the image functions being an example).

function_exists() returns false for functions disabled through disable_function in php.ini.

Via the discussion on #26772, it seems like it's possible for a false positive when using suhosin config to disable. We added an additional check for ini_get( 'disable_functions' ) in [29330], but I'm not sure how that works with suhosin anyway, which uses the option suhosin.executor.func.blacklist.

It may be that we've done just fine with function_exists() on it's own beyond that one bug report, but I may also not understand a piece.

[dd32]

Replying to jeremyfelt:

Replying to jdgrimes:

@dd32 function_exists() ​doesn't detect disabled functions:

Note:
A function name may exist even if the function itself is unusable due to configuration or compiling options (with the image functions being an example).

function_exists() returns false for functions disabled through disable_function in php.ini.

Via the discussion on #26772, it seems like it's possible for a false positive when using suhosin config to disable. We added an additional check for ini_get( 'disable_functions' ) in [29330], but I'm not sure how that works with suhosin anyway, which uses the option suhosin.executor.func.blacklist.

It may be that we've done just fine with function_exists() on it's own beyond that one bug report, but I may also not understand a piece.

Technically you're right, personally though, I don't want to perform silly work arounds like that. I'm fine with a server which uses a hardening extension to disable a safe function throwing warnings.

[jeremyfelt]

Replying to dd32:

Technically you're right, personally though, I don't want to perform silly work arounds like that. I'm fine with a server which uses a hardening extension to disable a safe function throwing warnings.

+1

[dd32]

In 38460:

Bootstrap: Check that ini_get_all() exists before calling it, allows us to work around hosts who disable the function for "security purposes".

Merges [38431] to the 4.6 branch.
Fixes #37680.