Out-Of-Memory when matching exec-like functions

[WhiteWinterWolf]

The default rules matching the exec-like functions (tested with exec() and system()) crashes PHP with an Out-Of-Memory error.

Using the following minimal test file directly invoked from the web root:

<?php
$gs = exec( 'foo' );
echo "YES";

The default rule:

sp.disable_function.function("exec").param("command").value_r("[$|;&`\\n\\(\\)\\\\]").drop();

Generates the following error:

snuffleupagus[8497]: [snuffleupagus][0.0.0.0][regexp][log] Something went wrong with a regexp (-51). in /█████/test.php on line 4
syslogd: last message repeated 6222 times
httpd[8497]: PHP Fatal error:  Out of memory (allocated 2097152) (tried to allocate 4096 bytes) in /██████/test.php on line 4

This issue is not linked to the regex or even anything actually related to the parameter, as replacing this rule by:

sp.disable_function.function("exec").param("command").value("bar").drop();

or even:

sp.disable_function.function("exec").param("foo").value("bar").drop();

Generates even worse Out-Of-Memory errors, this time uncontrolled by PHP: the HTTP process goes from around 100 MB to over 1 GB, depending what limit is imposed by the OS, and finally gets killed by the OS, producing either:

mmap() failed: [12] Cannot allocate memory

or more often:

[core:notice] [pid 36478:tid AH00052: child pid 37333 exit signal Illegal instruction (4)

error messages.

Commenting-out these rules in Snuffleupagus configuration removes the issues.

Other rules seem to work OK, including regex rules matching on SQL requests for instance which use the same syntax. I have therefore the impression that this issue is linked to some specific behavior or implementation of these exec-like functions.

I'm using FreeBSD with PHP as an Apache module, latest packages version: php80-8.0.6, php80-snuffleupagus-0.7.0.

[jvoisin]

The memory leak should be fixed via db14b85, I'm currently looking at why it's trying to match so many times.

I also landed 194b0bc, which might help memory-wise.

[jvoisin]

I add your testcase in ad623f1 but it doesn't reproduce in the CI. Can you confirm that it should be a faithful reproducer of your problem?

[WhiteWinterWolf]

I'm investigating for other things which may be involved in this issue.

Note however that I do not think that this issue is linked to PCRE, as it happens no matters if the parameter name actually exists or not (using param("foo") also triggers the issue), and no matter if you use match or match_r on it.

[WhiteWinterWolf]

I got the "why", but don't ask me about the "how" ;) .

Seemingly Snuffleupagus doesn't like when its configuration file is set from the Apache configuration file using the php_admin_value and php_value statements:

• When using only php.ini to set sp.configuration_file, this issue cannot be reproduced.
• When using php_value sp.configuration_file "/path/to/rules", the given file is simply ignored (abnormal behavior), the local value for the sp.configuration_file setting just reflects the master one set in php.ini, and the current issue cannot be reproduced.
• When using php_admin_value sp.configuration_file "/path/to/rules", the master and local values show the value defined respectively in the php.ini and in the Apache configuration files (as expected). This is when the issue appears. Maybe Snuffleupagus relies somewhere on the assumption that the local and the master values for sp.configuration_file are the same ?

Knowing this, I have remade the layout of my php.ini files to avoid the issue and will add a drop rule preventing runtime overwrite of the sp.configuration_file setting (even-though, as per my understanding, Snuffleupagus currently only use it only on startup). This is sufficient to solve the issue in my environment.

Nevertheless, I don't know whether there is any plan to support the php_value and php_admin_value ?

• If it is not supported, it should be documented and I think that the fix could be as simple as raising a descriptive fatal error if the local and master value are not the same for sp.configuration_file.
• If it is supported, Snuffleupagus should be modified to only use on the local value.

For me the first choice seem both the easiest and the safest, as it should also prevent an attacker from potentially disabling Snuffleupagus through .htaccess or .user.ini files.

[WhiteWinterWolf]

I didn't went deep in Snuffleupagus source code, but as per my understanding snuffleupagus.c defines a callback function OnUpdateConfiguration() which is invoked by PHP when loading the configuration, and this callback is in charge of expanding globs, parsing each configuration file and attach hooks.

If the sp.configuration_file is successively set first to a master then to a local value, chances are that this callback function will be called two times with two different set of configuration files. I wonder what may happen in this circumstance, in particular if Snuffleupagus may not try to re-hook already hooked functions (ie. hook its own functions), which may effectively result in infinite loops and Out-Of-Memory errors.

BTW, the way strtok_r() is used in snuffleupagus.c seems weird to me. As per my understanding, the last argument shouldn't be the parsed string but a distinct pointer used to keep a status information (and I'm not sure either why you are using the recursive version of strtok()).

It is also worth noting that this situation where Snuffleupagus has to handle different local and master values will also be present in #260.

[jvoisin]

I removed strtok_r usage in d934db8, and I'm looking at recursion issues, albeit this is weird, because there are guards everywhere to prevent this from happening :/

[jvoisin]

8353de0 might™ prevent the issue, can you test @WhiteWinterWolf ?
Otherwise, I'll try install freebsd and apache2 to reproduce this issue this weekend :)

[WhiteWinterWolf]

Sadly 8353de0 doesn't seem to have any effect on the issue :(

Reproduced on another environment, I can confirm it happens when I use the php_admin_value sp.configuration_file directive in Apache configuration in addition to the sp.configuration_file directive in php.ini, no matter the patch above is applied or not. As soon as I comment-out the setting in the Apache configuration, the issue disappears.

The only consequences I can see for now for Snuffleupagus in the presence of this settings:

• The sp.configuration_file is associated to two different values as "local" and "master" settings.
• PHP invokes the OnUpdateConfiguration() twice, once for each value.

What I've seen is that the issue doesn't come directly from the fact that the "master" and "local" value are different: as long as the globing results in the very same files to be loaded, there is no issue. The issue happens when the php_admin_value directive causes Snuffleupagus to load an additional configuration file, no matter its content as even loading a single empty file at this step make the issue to appear.

[WhiteWinterWolf]

In case it might be useful, I've added log messages when entering and quittting ZEND_FUNCTION(check_disabled_function):

--- sp_disabled_functions.c.orig        2021-01-02 18:22:07 UTC
+++ sp_disabled_functions.c
@@ -490,6 +490,7 @@ static void should_drop_on_ret(const zva
 }

 ZEND_FUNCTION(check_disabled_function) {
+  sp_log_err("debug", "Entering '%s'.", __func__);
   zif_handler orig_handler;
   const char* current_function_name = get_active_function_name(TSRMLS_C);

@@ -508,6 +509,7 @@ ZEND_FUNCTION(check_disabled_function) {
           .config_disabled_functions_reg_ret->disabled_functions,
       SNUFFLEUPAGUS_G(config).config_disabled_functions_ret_hooked,
       execute_data);
+  sp_log_err("debug", "Exiting '%s'.", __func__);
 }

 static int hook_functions_regexp(const sp_list_node* config) {

The normal behavior, when there is no issue, produces the following logs:

May 16 21:33:21 freebsd snuffleupagus[14706]: [snuffleupagus][0.0.0.0][debug][log] Entering 'zif_check_disabled_function'. in /var/www/main/data/index.php on line 4
May 16 21:33:21 freebsd snuffleupagus[14706]: [snuffleupagus][0.0.0.0][debug][log] Exiting 'zif_check_disabled_function'. in /var/www/main/data/index.php on line 4

=> A single entry immediately followed by a single exit.

The erroneous behavior produces the following logs:

May 16 21:33:32 freebsd snuffleupagus[14747]: [snuffleupagus][0.0.0.0][debug][log] Entering 'zif_check_disabled_function'. in /var/www/main/data/index.php on line 4
May 16 21:33:45 freebsd syslogd: last message repeated 521165 times
May 16 21:33:45 freebsd php: PHP Fatal error:  Allowed memory size of 134217728 bytes exhausted (tried to allocate 4096 bytes) in /var/www/main/data/index.php on line 4

=> Infinite entry until memory exhaustion, no exit.

This gives the impression that the recursion happens here.

[WhiteWinterWolf]

The last line being executed from this function being orig_handler(INTERNAL_FUNCTION_PARAM_PASSTHRU), then we get back on the entering message.

[WhiteWinterWolf]

I also took a look on the name and address of the function being hooked in this function:

• Normal behavior, one single occurrence of the following message:

Entering 'zif_check_disabled_function'.
    current_function_name = 'exec' 
    orig_handler = 0x803215ff0 
Exiting 'zif_check_disabled_function'.

• Erroneous behavior, the log is cycling over the following messages:

Entering 'zif_check_disabled_function'.
     current_function_name = 'exec'
     orig_handler = 0x8050226f0

We can see that while in both cases the name of the targeted function remains correct, exec, its pointer value changes (I ensured that, with identical settings, the pointer value remains the same across several Apache restarts).

With a bit of further reverse engineering, my understanding is that this pointer value is defined in hook_function. I've added a debug message in it:

--- sp_utils.c.orig     2021-01-02 18:22:07 UTC
+++ sp_utils.c
@@ -404,6 +404,7 @@ int hook_function(const char* original_n

   if ((func = zend_hash_str_find_ptr(CG(function_table),
                                      VAR_AND_LEN(original_name)))) {
+    sp_log_err("debug", "hook_function: '%s' = %p, new: %p", original_name, func->handler, new_function);
     if (func->handler == new_function) {
       return SUCCESS;  // the function is already hooked
     } else {

It shows that there is no issue during the initialization phase, as both the sane and bogus Apache settings produce the exact same output, showing two call to this function (I would expect a single call, I don't know if this is normal, but the same behavior can also be seen for all other hooked functions):

hook_function: 'exec' = 0x803215ff0, new: 0x8050225b0 in [no active file] on line 0
hook_function: 'exec' = 0x8050225b0, new: 0x805022710 in [no active file] on line 0

However, as soon as PHP handles an incoming request invoking the hooked exec:

• The sane configuration only outputs the Entering 'zif_check_disabled_function'. stuff as seen above.
• While the erroneous configuration re-hooks again all functions (with three lines for each function) before engaging in the infinite loop:

hook_function: 'exec' = 0x805022710, new: 0x8050225b0 in [no active file] on line 0
hook_function: 'exec' = 0x8050225b0, new: 0x805022710 in [no active file] on line 0
hook_function: 'exec' = 0x805022710, new: 0x805022710 in [no active file] on line 0

Once all functions have been re-hooked comes the loop:

Entering 'zif_check_disabled_function'. in /var/www/main/data/index.php on line 4
     current_function_name = 'exec' in /var/www/main/data/index.php on line 4
     orig_handler = 0x805022710 in /var/www/main/data/index.php on line 4
Entering 'zif_check_disabled_function'. in /var/www/main/data/index.php on line 4
    current_function_name = 'exec' in /var/www/main/data/index.php on line 4
    orig_handler = 0x805022710 in /var/www/main/data/index.php on line 4
...

Sadly, 0x805022710 is not the pointer to the original PHP function (0x803215ff0), but the pointer to Snuffleupagus' own zif_check_disabled_function which was set during the initial hook session, hence the recursive call and the out-of-memory crash.

Next step: why does Snuffleupagus attempt to reload its configuration / re-hook its functions?

I don't know yet, but as a reminder this issue occurs only when hooking exec-like functions, rules about hooking mysqli functions for instance work with no issue in every case. I can just guess that there is something special with these functions, maybe because of a fork or something like that.

[xXx-caillou-xXx]

Hi, I'll look into it sometime next week o/

[DanielRuf]

Any updates regarding this or any help needed?

[jvoisin]

Can you try with the current -git version? A lot of code was changed since this issue was opened.

[WhiteWinterWolf]

Thanks @jvoisin, I will check this as soon as I can and keep you informed.