By jvoisin

While polishing snuffleupagus for its first release, my intern and I encountered an “interesting” issue: our hardened random feature was failing in certain environnements, and we struggled to understand why.

To spice things up, we only managed to reproduce it locally when there was no debug symbols in the binary, on an archlinux machine.

Here is the function in charge of everything:

/* This function is needed because `rand` and `mt_rand` parameters
 * are optional, while the ones from `random_int` aren't. */
static void random_int_wrapper(INTERNAL_FUNCTION_PARAMETERS) {
  zend_long min, max, result;

switch (EX_NUM_ARGS()) {
case 0:
  min = 0;
  max = PHP_MT_RAND_MAX;
  break;
case 1:
  // LCOV_EXCL_BR_START
  ZEND_PARSE_PARAMETERS_START_EX(ZEND_PARSE_PARAMS_QUIET, 1, 1);
  Z_PARAM_LONG(min);
  ZEND_PARSE_PARAMETERS_END();
  // LCOV_EXCL_BR_END
  max = PHP_MT_RAND_MAX;
  break;
case 2:
default:
  ZEND_PARSE_PARAMETERS_START_EX(ZEND_PARSE_PARAMS_QUIET, 0, 2);
  Z_PARAM_LONG(min);
  Z_PARAM_LONG(max);
  ZEND_PARSE_PARAMETERS_END();
}

if (min > max) {
  if (php_random_int_throw(max, min, &result) == FAILURE) {
    return;
  }
} else {
  if (php_random_int_throw(min, max, &result) == FAILURE) {
    return;
  }
}

RETURN_LONG(result);
}

We started by checking that it was called when rand was used from PHP, and it was indeed the case. We ran the failing test in GDB (you might want to grab PHP’s .gdbinit and maybe peda too, to improve your user experience) to investigate further.

Since GDB struggled to find the function, we couldn’t put a breakpoint on it. Since we were too lazy to use an other tool to find its offset, we added a good ol’ __asm__(“int3”): right after its entry, to trigger a SIGTRAP, giving us a way to investigate what’s going on.

It seemed that the function was returning early. This is weird, because there is only a return at the end of it, and two others that are after calls to php_random_int_throw that would print a message before returning if something went wrong (hence the throw in the name).

So we took at look at its control flow graph using radare2:

                                <@@@@@@>
                                     f t
                              ┌──────┘ └──┐
                              │           │
                              │           │
                         __5574__      __5600__
                              f t       v
                         ┌────┘ └───────│────┐
                         │              │    │
                         │              │    │
                    __5579__            │    │
                       t f       ┌──────┘    │
                  ┌────┘ └─┐     │           │
                  │        │     │           │
                  │        │     │           │
               __5690__    │     │           │
                v          │     │           │
                │   ┌──────┘     │           │
                │   │            │           │
                │   │            │           │
               __5589__          │           │
                    f t          │           │
             ┌──────┘ └──┐       │           │
             │           │       │           │
             │           │       │           │
        __5591__      __5670__   │           │
         v                 f t   │           │
         └───┐             │ └───────────────┤
             │             │     │           │ 
             │             │     │           │
             │        __5681__   │           │
             │         v         │           │
             └─┌───────┘         │           │
               │                 │           │
               │                 │           │
              __5598__           │           │
                   f t           │           │
            ┌──────┘ └──┬────────┘           │
            │           │                    │
            │           │                    │
       __55a1__      __560a__                │
          t f           t f                  │
   ┌──────┘ └────┬──────┘ └─────┐            │
   │             │              │            │
   │             │              │            │
__5648__      __55c2__      __5624__         │
 v             v                v            │
 └─────────────────────────┬────┴────────────┘
                           │
                           v
                        __55ef__

There are 3 arrows at the end that are going to the terminal node, but oddly enough, there are 2 other ones that are doing the same. It seems that there is something fishy in the ZEND_PARSE_PARAMETERS_START_EX macros.

#define ZEND_PARSE_PARAMETERS_START_EX(flags, min_num_args, max_num_args) do { \
     const int _flags = (flags); \
     int _min_num_args = (min_num_args); \
     int _max_num_args = (max_num_args); \
     int _num_args = EX_NUM_ARGS(); \
     int _i; \
     zval *_real_arg, *_arg = NULL; \
     zend_expected_type _expected_type = IS_UNDEF; \
     char *_error = NULL; \
     zend_bool _dummy; \
     zend_bool _optional = 0; \
     int error_code = ZPP_ERROR_OK; \
     ((void)_i); \
     ((void)_real_arg); \
     ((void)_arg); \
     ((void)_expected_type); \
     ((void)_error); \
     ((void)_dummy); \
     ((void)_optional); \
     \
     do { \
         if (UNEXPECTED(_num_args < _min_num_args) || \
             (UNEXPECTED(_num_args > _max_num_args) && \
              EXPECTED(_max_num_args >= 0))) { \
             if (!(_flags & ZEND_PARSE_PARAMS_QUIET)) { \
                 zend_wrong_parameters_count_error(_flags &        ZEND_PARSE_PARAMS_THROW, _num_args, _min_num_args, _max_num_args); \
             } \
             error_code = ZPP_ERROR_FAILURE; \
             break; \
         } \
         _i = 0; \
         _real_arg = ZEND_CALL_ARG(execute_data, 0);

After a lot of exploration (protip: ctags is your friend.), we didn’t find anything suspicious that would trigger an early return. Notice how the do { aren’t closed: this is why this macro has a friend called #define ZEND_PARSE_PARAMETERS_END() that you must call when you’re done parsing arguments. It’s looking like this:

#define ZEND_PARSE_PARAMETERS_END_EX(failure) \
    } while (0); \
    if (UNEXPECTED(error_code != ZPP_ERROR_OK)) { \
        if (!(_flags & ZEND_PARSE_PARAMS_QUIET)) { \
            if (error_code == ZPP_ERROR_WRONG_CALLBACK) { \
                zend_wrong_callback_error(_flags & ZEND_PARSE_PARAMS_THROW, E_WARNING, _i, _error); \
            } else if (error_code == ZPP_ERROR_WRONG_CLASS) { \
zend_wrong_parameter_class_error(_flags & ZEND_PARSE_PARAMS_THROW, _i, _error, _arg); \
            } else if (error_code == ZPP_ERROR_WRONG_ARG) { \
zend_wrong_parameter_type_error(_flags & ZEND_PARSE_PARAMS_THROW, _i, _expected_type, _arg); \
            } \
        } \
        failure; \
    } \
} while (0)
 
#define ZEND_PARSE_PARAMETERS_END() \
    ZEND_PARSE_PARAMETERS_END_EX(return)

Still nothing suspiciou… wait. The parameter of the macro is used as a statement near the end of it! This is where our return comes from!

The (ghetto) solution is the call ZEND_PARSE_PARAMETERS_END_EX((void)0) instead of ZEND_PARSE_PARAMETERS_END, which is exactly what was done.

If you know a better way to do it, please tell us!

Until next time, see you in your php stack 😉

Equipe Sécurité
Equipe Sécurité
L'équipe sécurité de NBS System réalise des tests et audits de sécurité, mais fait également de la R&D autour de la sécurité informatique. Créateurs de NAXSI, CerberHost ou encore Snuffleupagus, ils sont très investis dans le milieu de l'Open Source.