Artificial truth

Yesterday, the SPIP team released SPIP 4.1.8, fixing a critical security issue. The bug in question was introduced by 4b83cb23ccbaa433fedc51040479230115bb4b5c, the 17^th of Match 2010. SPIP uses eval for its templating system, so being able to inject <?php in it is game over: this is exactly what happened here and it was patched like this:

function protege_champ($texte) {
    if (is_array($texte)) {
        return array_map('protege_champ', $texte);
    } elseif ($texte === null) {
        return $texte;
    } elseif (is_bool($texte)) {
        return $texte ? '1' : '';
    } elseif (is_string($texte) and $texte) {
-       if (preg_match(',^[abis]:\d+[:;],', $texte) and @unserialize($texte) !== false) {
-           // ne pas corrompre une valeur serialize
-           return $texte;
-       } elseif (strpbrk($texte, "&\"'<>") !== false) {
+       if (strpbrk($texte, "&\"'<>") !== false) {
            return spip_htmlspecialchars($texte, ENT_QUOTES);
        }
    }

    return $texte;
}

The exploit is obvious: pass a string that can be unserialized, containing your payload in php, like s:22:"<?php system('id');?>";. Various people have been sitting on this one since years.

Amusingly, there is a denial of service that was always present for sufficiently nested arrays, because of the recursive call, on which php can choke with a segfault.

What is more worrying, is that the patch is making use of the strpbrk, which can be made to return false should its first parameter contain a NULL byte before the characters being looked for. But there is a twist: SPIP has a "security screen", a simili web application firewall, a single file that can be updated independently, to mitigate known vulnerabilities, giving the time to the administrator to upgrade at a later point.

It contains this particular check, preventing us from injecting NULL-bytes:

if (strpos(
    (function_exists('get_magic_quotes_gpc') and @get_magic_quotes_gpc())
        ? stripslashes(serialize($_REQUEST))
        : serialize($_REQUEST),
    chr(0)
) !== false) {
    $ecran_securite_raison = "%00";
}

This is quite brittle, since the format used by serialize is more or less opaque, and could absolutely change its representation to encode NULL byte at some point. Interestingly, serialized classes do contain NULL-bytes, but more on this later.

Anyway, this is the snippet add to the security screen to mitigate the vulnerability:

if (
    isset($_REQUEST['formulaire_action_args'])
) {
    foreach ($_REQUEST as $k => $v) {
        if (is_string($v)
          and strpos($v, ':') !== false
          and strpos($v, '"') !==false
          and preg_match(',[bidsaO]:,', $v)
          and @unserialize($v)) {
            $_REQUEST[$k] = htmlentities($v);
            if (isset($_POST[$k])) $_POST[$k] = $_REQUEST[$k];
            if (isset($_GET[$k])) $_GET[$k] = $_REQUEST[$k];
        }
    }
}

A couple of things are standing out:

1. strpos could be bypassed by NULL-byte injection, should this snippet be placed before the aforementioned one. This isn't the case.
2. The needle argument in preg_match doesn't include all the possible type specifiers, so a payload like oubli=C:11:"HashContext":21:{<?php system('id');?>} would bypass it, albeit it wouldn't be executed since it doesn't match preg_match(',^[abis]:\d+[:;],', $texte).
3. The call to unserialize is scary, if only for the big fat red warning in PHP's documentation: "Do not pass untrusted user input to unserialize()". Fortunately, there is not a single call to magic methods in the whole SPIP core: no __wakeup, __destruct, …… but some extensions are making use of those. Oh, and also, odds are that there are still some exploitable memory corruptions in it ;)

This is all super-brittle, and I've been told that more low-hanging unauthenticated RCE are lurking in the dark. The right move would be to move away from SPIP, but this isn't always possible. Then there is of course snuffleupagus, but given the architecture of SPIP, it's non-trivial to mitigate things there, but all hope is not lost: One can use an allow list in eval, with something like sp.eval_whitelist.list("strlen,strcmp,echo"); to prevent code execution via template injection. Moreover, dangerous functions like system, shell_exec, …… should have their calls logged via sp.disable_function.function("system").simulation();. But keep in mind that bypasses are both plenty and not particularly hard. For this particular vulnerability, sp.unserialize_hmac.enable(); does mitigate it, by side-effect.

Thanks to Laluka for being my rubber duck debugger when it comes to SPIP.