[wp-trac] [WordPress Trac] #53295: Serialized data should be handled as an opaque value

[WordPress Trac]

#53295: Serialized data should be handled as an opaque value
-----------------------------+------------------------------
 Reporter:  whitewinterwolf  |       Owner:  (none)
     Type:  defect (bug)     |      Status:  new
 Priority:  normal           |   Milestone:  Awaiting Review
Component:  General          |     Version:
 Severity:  normal           |  Resolution:
 Keywords:  has-patch        |     Focuses:
-----------------------------+------------------------------

Comment (by siliconforks):

 I see at least two potential security vulnerabilities with
 [https://github.com/WordPress/wordpress-develop/pull/1312 PR #1312].

 == Object injection with PHP 5.6

 On PHP 5.6, object injection and code execution could occur inside the new
 implementation of `is_serialized()` because it is calling `unserialize()`.
 For example, consider a WordPress site running PHP 5.6 that has a plugin
 containing the `Example1` class from https://owasp.org/www-
 community/vulnerabilities/PHP_Object_Injection ...

 {{{#!php
 <?php
 class Example1
 {
    public $cache_file;

    function __construct()
    {
       // some PHP code...
    }

    function __destruct()
    {
       $file = "/var/www/cache/tmp/{$this->cache_file}";
       if (file_exists($file)) @unlink($file);
    }
 }
 }}}

 An attacker could simply do the following:

 1. Register as an ordinary (subscriber) user on the site.
 2. Log in and go to the "Profile" page.
 3. Enter serialized data in a profile field - e.g., in the first name
 field enter
 `O:8:"Example1":1:{s:10:"cache_file";s:15:"../../index.php";}`.
 4. Click "Update Profile" and WordPress will call `is_serialized()` on the
 malicious user input.  An instance of the `Example1` class will be created
 and its destructor will be called.  This will delete the file
 `/var/www/index.php`.

 == Object injection from data stored in the database by older WordPress
 versions

 The PHP 5.6 vulnerability above might not seem like too much of a problem
 because WordPress will likely be dropping support for PHP 5.6 soon, and
 this PR could be merged after that happens.  However, there is still
 another, more subtle vulnerability, which is what @nacin described in
 #17375 - it is not possible to safely change the behavior of
 `is_serialized()`.

 Suppose this PR were to be accepted into a future WordPress version, say,
 WordPress 5.8.  Then a malicious user could do the following:

 1. Find a WordPress site running a version of WordPress earlier than
 version 5.8.
 2. Register as a user on that site.
 3. Log in and go to the "Profile" page.
 4. Enter a serialized `C:...` value the first name field.
 5. Click "Update Profile" and this `C:...` value will be stored in the
 database.

 Note: at this point you may be thinking, "Wait, what does this have to do
 with the PR in question?  If this attack requires an older version of
 WordPress without the PR, then it must be a vulnerability in that version
 of WordPress.  It has nothing to do with this PR."  But there isn't
 actually any vulnerability at this point - the `C:...` value will never be
 unserialized by WordPress.  If WordPress runs `is_serialized()` on this
 value, the function will return false, so WordPress will never actually
 consider it to be serialized data at all, and it will never try to
 unserialize it.  WordPress just thinks that the attacker is a user whose
 first name happens to be `C:...` and it will simply display that string on
 the user's profile page.

 The vulnerability will arise in the future if the site ever upgrades to
 WordPress 5.8 (with the new `is_serialized()` implementation).  Then, if
 WordPress ever attempts to retrieve the attacker's first name from the
 database, the `is_serialized()` function will return true for this value,
 so WordPress will then attempt to unserialize it, potentially resulting in
 code execution.

 I suspect that a fix for #53299 might result in the same vulnerability.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/53295#comment:8>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform