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:
ZEND_VM_HANDLER(73, ZEND_INCLUDE_OR_EVAL, CONST|TMPVAR|CV, ANY, EVAL) { // […] new_op_array = zend_include_or_eval(inc_filename, opline->extended_value); FREE_OP1(); 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)) { ZEND_VM_ENTER(); } else { ZEND_ADD_CALL_FLAG(call, ZEND_CALL_TOP); zend_execute_ex(call); zend_vm_stack_free_call_frame(call); } // […] }
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); efree(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 gdb-peda$
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 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) { is_in_eval_and_whitelisted(execute_data); EX(func)->internal_function.handler(INTERNAL_FUNCTION_PARAM_PASSTHRU); if (UNEXPECTED(NULL != orig_zend_execute_internal)) { orig_zend_execute_internal(INTERNAL_FUNCTION_PARAM_PASSTHRU); } }
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.).