Artificial truth

I just published a new release of Snuffleupagus, the hardening module for php7+ and php8+, version 0.14.0, codename "Blumenbach", named after Johann Friedrich Blumenbach, the scientist who gave the woolly mammoth its first scientific name, Elephas primigenius (first-born elephant), in 1799.

There aren't any new flashing features, only bug fixes and improvements, better PHP85 support, a fresh hardening ruleset for SPIP because people keep burning RCE for it, and also a bug fix that was so tedious that I'm going to ramble about it here.

PHP 8.5 shipped with an innocuous-looking optimizer improvement: opcache can now inline trivial user functions. For example, functions that are taking no arguments and are returning a constant are completely removed with the constant. No more INIT_FCALL, no DO_UCALL, no function call at all. Performance-wise, this is pretty cool. Less so when our beloved extension hooks zend_execute_ex to inspect return values.

Let's have an example. Imagine someone wrote a rule like sp.disable_function.function("get_role").ret("admin").drop();, dropping the execution if the get_role function returns admin. Under the hood, Snuffleupagus replaces zend_execute_ex with its own wrapper. When a user function returns, the wrapper checks the return value against the configured rules. On PHP 8.5 with opcache enabled, a function like function get_role() { return "admin"; } would compile to INIT_FCALL "get_role"; V0 = DO_UCALL, …, but after opcache's optimizer runs, it simply becomes ECHO string("admin"). The function call is gone. zend_execute_ex is never invoked. The security rule never fires. The application runs as if the rule didn't exist.

The obvious-ish fix would be to use PHP's observer API introduced in PHP: zend_observer_fcall_register. It integrates with the VM and is supposed to work alongside opcache and JIT. But in our case, they're useless: the observer init callback is invoked the first time a function is executed. If the function is never executed, because the optimizer replaced the call with a constant, the observer never sees it.

The inlining decision happens in opcache's zend_try_inline_call(), containing several gating conditions:

    if (func->type == ZEND_USER_FUNCTION
     && !(func->op_array.fn_flags & (ZEND_ACC_ABSTRACT|ZEND_ACC_HAS_TYPE_HINTS|ZEND_ACC_DEPRECATED|no_discard))
        /* TODO: function copied from trait may be inconsistent ??? */
     && !(func->op_array.fn_flags & (ZEND_ACC_TRAIT_CLONE))
     && fcall->extended_value >= func->op_array.required_num_args
     && func->op_array.opcodes[func->op_array.num_args].opcode == ZEND_RETURN)
        // inline the call

Most of those conditions can't really be influenced without breaking everything. The one we can twiddle with is the (ZEND_ACC_ABSTRACT|ZEND_ACC_HAS_TYPE_HINTS|ZEND_ACC_DEPRECATED|no_discard) one. So the whole problem collapses into a smaller one: which one of those flags can we set on the functions we care about?

PHP's Zend extension API provides an op_array_handler callback that fires during pass_two(), right after initial compilation but before opcache optimization. This is the perfect hook point: we can see the function, check if it has return-value rules, and set a flag that prevents inlining.

I picked ZEND_ACC_HAS_TYPE_HINTS: the runtime impact is negligible as it only causes the VM to execute ZEND_RECV opcodes instead of skipping them. But because functions eligible for inlining are trivial constant-return functions (at least for now), they typically have zero arguments, meaning zero RECV opcodes to execute. The implementation is straightforward. In sp_op_array_handler, for each compiled user function, check if its name matches a return-value rule, and if so, flip the flag: if (has_ret_rule) { op->fn_flags |= ZEND_ACC_HAS_TYPE_HINTS; }.

Of course, the day that PHP decides that ZEND_ACC_HAS_TYPE_HINTS isn't a blocker for inlining functions, I'll have to find something else.

While we're on the topic of tedious things, an issue was opened about a 2x performance impact when using Snuffleupagus on a WordPress, ouch. The culprit turned out to be "chained" rules, the ones written as sp.disable_function.function("system>base64_decode") to only forbid a function when it's called from another one.

The catch is that having even a single one of them flips on the global execution hook, meaning every single function call in the whole application, userland and internal, gets funnelled through it. For each of those calls Snuffleupagus then walked the entire list of chained rules, and for every rule the whole callstack was reconstructed. On a request that does hundreds of thousands of function calls, that adds up fast.

The fix is boring in the best way: chained rules now live in the same per-function hashtable as regular ones, keyed by the innermost function of the chain: a call to base64_decode only ever looks at the rules that actually mention base64_decode, and everything else pays nothing. As a bonus, the hot path no longer rebuilds the current filename on every call when nothing can possibly match. WordPress is fast again. Well, as fast as WordPress can go anyway.

Changelog

• Improved compatibility with PHP 8.5, including a fix for opcache inlining bypassing return-value rules
• Fix a ~2x slowdown caused by chained disable_function rules
• New hardening ruleset for SPIP
• Add coverage for yet another disable_functions bypass
• Fix a type confusion and a null-pointer dereference in the cookie-encryption code
• Reduce the lifetime of cryptographic material in memory
• Harden configuration parsing: reject non-ASCII characters in rules, show the config filename in errors, and stop parsing the config twice under Apache/php-fpm reloads.
• Fix phpinfo() reporting "enabled" when no configuration is loaded
• Improve upload_validation robustness (fork-failure handling, correct pid waiting, strtok_r misuse)
• Fix a truncation issue in the unserialization handling code
• Fix half a dozen minor memory leaks
• A bunch of code correctness / simplification / hardening changes, and a handful of new tests

Oh and also, somebody told me about a by-design virtual-patching bypass, provided an attacker already has arbitrary PHP code execution, so I made it public.

As usual, if you want to help, we have some low hanging fruits ♥

See you in your PHP stack!