Implementing an `eval` allow/blocklist in Snuffleupagus
Fri 22 March 2019 — download

A while ago, a blocklist/allowlist for eval à la Suhosin was implemented in Snuffleupagus. Actually, the implement goes a bit further than the suhosin one: it's possible to allow/blocklist user-defined functions too, and not only builtins.

In php7, eval isn't a function anymore, it's a language construct, handled via an opcode called INCLUDE_OR_EVAL, (kind of) implemented in zend_vm_def.h:

  // […]
  new_op_array = zend_include_or_eval(inc_filename, opline->extended_value);
        if (UNEXPECTED(EG(exception) != NULL)) {
                // […]]
        } else if (EXPECTED(new_op_array != NULL)) {
                    // […]
                i_init_code_execute_data(call, new_op_array, return_value);
                if (EXPECTED(zend_execute_ex == execute_ex)) {
                } else {
                        ZEND_ADD_CALL_FLAG(call, ZEND_CALL_TOP);
  // […]

It's calling zend_include_or_eval, which has a case dedicated to eval:

case ZEND_EVAL: {
    char *eval_desc = zend_make_compiled_string_description("eval()'d code");
    new_op_array = zend_compile_string(inc_filename, eval_desc);

Unfortunately, zend_compile_file, as its name states, only compiles the php source code, it doesn't execute it. We could hook it and inject an eval_start and eval_stop markers in the compiled code, but this is a bit ugly.

Instead, we could hook zend_execute_ex, increment a counter if the type of the current op_array is ZEND_EVAL_CODE, call the original zend_execute_ex function, check the counter in the hook, and then decrement the counter after the execution of zend_execute_ex.

Can you guess why we're using a counter instead of a boolean?

So here we go; we implemented this, added some tests, and, … some of them were failing: builtin functions couldn't be allow/blocklisted. This could be walked around by hooking them all one by one and checking the eval counter in the hook, but there must be a better way.

Most of the time, when dealing with issues like this one, it's usually easier to use GDB (with peda and php's own gdbinit) than reading the source code.

Since I'm not a nice person, I added an __asm__("int3") in the zend_execute_ex hook, in a condition checking if the eval counter flag is superior to zero, and relaunched the testsuite, with a test containing something like eval("cos(0.5)") and cos in the allowlist.

Step-step-step until hitting the zif_cos function (PHP automatically adds the zif prefix to functions), and this showed up:

gdb-peda$ dump_bt executor_globals.current_execute_data
[0x7ffff3814200] cos(0.500000) [internal function]
[0x7ffff3814170] (main) [internal function]
[0x7ffff3814030] (main) /home/jvoisin/Dev/snuffleupagus/src/tests/eval_whitelist_builtin.php:4
gdb-peda$ bt
#0  0x0000555555715e12 in zif_cos ()
#1  0x0000555555806b9d in ?? ()
#2  0x00005555557f7ddb in execute_ex ()
#3  0x00007ffff2d6162d in sp_execute_ex (execute_data=0x7ffff3814170) at /home/jvoisin/Dev/snuffleupagus/src/sp_execute.c:104
#4  0x000055555584972c in ?? ()
#5  0x00005555557f7ddb in execute_ex ()
#6  0x00007ffff2d6162d in sp_execute_ex (execute_data=0x7ffff3814030) at /home/jvoisin/Dev/snuffleupagus/src/sp_execute.c:104
#7  0x000055555584c617 in zend_execute ()
#8  0x00005555557b7203 in zend_execute_scripts ()
#9  0x00005555557560b0 in php_execute_script ()
#10 0x000055555584e2d0 in ?? ()
#11 0x000055555563885e in main ()
#12 0x00007ffff64123f1 in __libc_start_main (main=0x5555556383f0 <main>, argc=0x42, argv=0x7fffffffdd48, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffdd38) at ../csu/libc-start.c:291
#13 0x00005555556389aa in _start ()
gdb-peda$ disassemble 0x0000555555806b9d
No function contains specified address.
gdb-peda$ x/48ni 0x0000555555806b9d
   0x555555806b9d:      mov    rax,QWORD PTR [r13+0x38]
   0x555555806ba1:      mov    QWORD PTR [rbp+0x1e0],rax
   0x555555806ba8:      mov    ebx,DWORD PTR [r13+0x2c]
   0x555555806bac:      test   ebx,ebx
   0x555555806bae:      je     0x555555806bf0
   0x555555806bb0:      lea    r12,[r13+0x60]
   0x555555806bb4:      shl    rbx,0x4
   0x555555806bb8:      add    rbx,r12
   0x555555806bbb:      jmp    0x555555806bc5
   0x555555806bbd:      nop    DWORD PTR [rax]
   0x555555806bc0:      cmp    r12,rbx
   0x555555806bc3:      je     0x555555806bf0
   0x555555806bc5:      sub    rbx,0x10
   0x555555806bc9:      test   BYTE PTR [rbx+0x9],0x4
   0x555555806bcd:      je     0x555555806bc0
   0x555555806bcf:      mov    rdx,QWORD PTR [rbx]
   0x555555806bd2:      sub    DWORD PTR [rdx],0x1
   0x555555806bd5:      jne    0x555555806bc0
   0x555555806bd7:      mov    rdi,QWORD PTR [rbx]
   0x555555806bda:      mov    DWORD PTR [rbx+0x8],0x1
   0x555555806be1:      call   0x5555557b53a0 <_zval_dtor_func_for_ptr>
   0x555555806be6:      cmp    r12,rbx
   0x555555806be9:      jne    0x555555806bc5
   0x555555806beb:      nop    DWORD PTR [rax+rax*1+0x0]
   0x555555806bf0:      test   BYTE PTR [r13+0x2b],0x80
   0x555555806bf5:      jne    0x555555806cbb
   0x555555806bfb:      mov    QWORD PTR [rbp+0x1c8],r13
   0x555555806c02:      test   BYTE PTR [r15+0x1f],0x20
   0x555555806c07:      je     0x555555806c16
   0x555555806c09:      movsxd rax,DWORD PTR [r15+0x10]
   0x555555806c0d:      add    rax,r14
   0x555555806c10:      test   BYTE PTR [rax+0x9],0x4
   0x555555806c14:      jne    0x555555806c30
   0x555555806c16:      cmp    QWORD PTR [rbp+0x348],0x0
   0x555555806c1e:      jne    0x555555806c69
   0x555555806c20:      add    rsp,0x8
   0x555555806c24:      add    r15,0x20
   0x555555806c28:      pop    rbx
   0x555555806c29:      pop    rbp
   0x555555806c2a:      pop    r12
   0x555555806c2c:      pop    r13
   0x555555806c2e:      ret
   0x555555806c2f:      nop
   0x555555806c30:      mov    rcx,QWORD PTR [rax]
   0x555555806c33:      mov    edi,DWORD PTR [rcx]
   0x555555806c35:      lea    edx,[rdi-0x1]
   0x555555806c38:      test   edx,edx
   0x555555806c3a:      mov    DWORD PTR [rcx],edx

There is only one function, even if gdb doesn't detect it as such, between execute_ex and zif_cos, looking pretty small, only calling _zval_dtor_func_for_ptr, and with a weird-looking prologue: it suspiciously looks like a handler/callback function, so odds are that we should be able to hook it somehow.

I started to search for this function in PHP's source code when an other Snuffleupagus developer asked why we didn't simply hook zend_execute_internal and see what happens. Why indeed, because this is exactly the right function to hook. But why didn't this function appear in GDB?

$ git grep 'zend_execute_internal ='
Zend/zend.c:800:                        zend_execute_internal = dtrace_execute_internal;
Zend/zend.c:804:                        zend_execute_internal = NULL;
Zend/zend.c:810:        zend_execute_internal = NULL;
Zend/zend_execute_API.c:805:            if (EXPECTED(zend_execute_internal == NULL)) {

Likely because it's set to NULL and is apparently only used for debugging purposes. So it's never called, hence why it didn't show up in GDB.

Allow/blocklisting builtin functions can effectively be done in a few lines of code, by hooking zend_execute_internal:

static void sp_zend_execute_internal(INTERNAL_FUNCTION_PARAMETERS) {


  if (UNEXPECTED(NULL != orig_zend_execute_internal)) {

For the record, this is a similar approach to Suhosin's one. Why bothering to reimplement it then? First, because Suhosin is distributed under the PHP license while Snuffleupagus is under LGPL, so we can't simply copy/paste code. Second, because suhosin was written quite some time ago, and PHP changed some of its internal in the meantime, making some trick less effective when not plainly incompatible.

Interestingly, when taking a quick look at Suhosin's source code, we thought that they went for a completely different route, since the first match for eval_whitelist is in their own handler for function_exists, so we naively imagined that they put their check here because php is calling function_exist every single time before calling native function. Spoiler alert: Of course, PHP doesn't do that.

The reason why Suhosin is placing their first check (there is a second one in their zend_execute_internal handler) here is for consistency's sake, so developers could write something like eval("if(function_exists('system')) system('ls');"). We didn't bother doing something similar, because we already have enough code in Snuffleupagus, amongst other reasons.

But why bother implement a blocklist in the first place, since odds are that it'll be bypassable anyway? Because we're in a world where deploying an allowlist might break some existing websites, and sysadmin do prefer to have a working website than a secure-but-broken one. Deploying a blocklist containing system, base64_decode and their friends should help to detect kiddies attacks while not breaking previous websites (and if it does, something is terribly wrong in the first place.).