Wordpress and the notorious unserialize

Few weeks back we got the following on the php bug reporting system https://bugs.php.net/bug.php?id=75006

Unserialize must not be used on untrusted input.
We don't consider issues in unserialize as security vulnerabilities

This means it is a right time to ring a bell towards wp infosec community regarding wordpress and its maybe_serialize and maybe_unserialize , to be precise is_serialized “security” functions.

Before we jump to review those functions I’ll post here few simple facts about unserialize function:

  • “O” have its own alternative “o” for StdClass object.
  • “s” and “S” are almost the same, except “S” could handle hex input there — hello IDS/WAF bypass.
  • a, O, C, o, s, S could have + sign right after : e.g. a:+…, O:+… .
  • s:3:”grr”;testtest is valid serialized string (could be object, array too) that would be unserialized without problems.

you can get more informations here: https://github.com/php/php-src/blob/master/ext/standard/var_unserializer.c

How wordpress handles serialized input

Let us take a look in is_serialized wp “security” function. There we have the following checks / constraints:

if ( $strict ) {
$lastc = substr( $data, -1 );
if ( ';' !== $lastc && '}' !== $lastc ) {
return false;
}
} else {
$semicolon = strpos( $data, ';' );
$brace = strpos( $data, '}' );
// Either ; or } must exist.
if ( false === $semicolon && false === $brace )
return false;
// But neither must be in the first X characters.
if ( false !== $semicolon && $semicolon < 3 )
return false;
if ( false !== $brace && $brace < 4 )
return false;
}

=> is_serialized(“s:3:\”grr\”;testtest”, true) or is_serialized(“s:3:\”grr\”;testtest”) will return false .

case 's' :
if ( $strict ) {
if ( '"' !== substr( $data, -2, 1 ) ) {
return false;
}
} elseif ( false === strpos( $data, '"' ) ) {
return false;
}
// or else fall through
case 'a' :
case 'O' :
return (bool) preg_match( "/^{$token}:[0-9]+:/s", $data );

=> will consider a:+1:{…, O:+3:…, C & o in any form as not serialized strings e.g. will be written in the DB same like they are supplied.

Also maybe_serialize and maybe_unserialize aren’t recursive functions. This input:

$arr = array(
"key1" => 'a:2:{s:4:"key1";s:4:"val1";s:4:"key2";s:4:"val2";}',
"key2" => 'a:2:{s:4:"key1";s:4:"val1";s:4:"key2";s:4:"val2";}'
);

if passed trough maybe_serialize will become:

a:2:{s:4:"key1";s:50:"a:2:{s:4:"key1";s:4:"val1";s:4:"key2";s:4:"val2";}";s:4:"key2";s:50:"a:2:{s:4:"key1";s:4:"val1";s:4:"key2";s:4:"val2";}";}

and if we have repetitive or recursive maybe_unserialize call over the values of this serialized array then we are facing unserialize on the user input.

For the end is_serialized_string will return true on s:+1:”a”0serialized_payload and it is dangerous for older versions of PHP.

What not to do when handling serialized data in wordpress

From the above we have 5 rules in order to find unserialization of user supplied data in wp based products:

  • If you have insert/update DB statement towards column that could be maybe_unserialize-d always check with is_serialized($input, false).
  • Never trust the wp core that you can simple call @unserialize towards DB cell that is filled in via maybe_serialize .
  • Never perform repetitive or recursive maybe_unserialize towards DB cell that is filled in via maybe_serialize.
  • Never pass user input towards maybe_unserialize based on black list pattern — always use white list!
  • Never perform any DB tasks that would remove at least one character from maybe_serialize input.

I would not mention here performing @unserialize or maybe_unserialize on any direct user input like $_REQUEST variable, file content because it is more than obvious…
Having on mind this 5 rules if we apply the opposite we can search for unserialize vulnerabilities towards wp plugins/themes. This opens wide door for two types of attacks towards wp system:

  • Object injection towards existing loaded classes from wp setup plugins or PHP environment.
  • RCE, dumping memory based on bug in unserialize .

Proposed fix

Having on mind this approach will lower the possibility of this type of attack at minimum:

Described issues/features e.g. one way bypasses can be fixed in the wp core if there is stop check for the + payload in is_serialized and if maybe_serialize function is turned into recursive one.

Bonus for this writing

  • What would be the output of this PHP scripts? :)
$str = "s:+3:\"grr\";";
echo serialize(unserialize($str));

and

$arr = 'a:+2:{i:1;s:3:"key";i:0;o:1:"s:2:"ID";s:1:"1";}}';
echo serialize(unserialize($arr));
  • Will sanitize_email and sanitize_text_field prevent inserting valid PHP serialized string towards DB? :)
  • Are there interesting gadget chains in most popular plugins on the wp repository? :)

Announce

Based on the previous knowledge from fast search over the most popular wp plugins I got the following results wich will be bigger in much more detailed check:

Point by point will be explained in the series of blog posts in the next days.

Conclusion

Will be confirmed in the next period, but will be put here today.
If unserialize is considered as security issue while auditing PHP code, then why add_metadata, add_option, called with value from user input or “sanitized” by common wp functions shouldn’t be?

Promo

If you are wp developer or wp host provider or wp security product provider with valuable list of clients, we offer subscription list and we are exceptional (B2B only).