[wp-trac] [WordPress Trac] #53391: Unfreeze the code of is_serialized()
WordPress Trac
noreply at wordpress.org
Sat Jun 12 17:14:55 UTC 2021
#53391: Unfreeze the code of is_serialized()
-----------------------------+-----------------------------
Reporter: whitewinterwolf | Owner: (none)
Type: defect (bug) | Status: new
Priority: normal | Milestone: Awaiting Review
Component: General | Version:
Severity: normal | Keywords:
Focuses: |
-----------------------------+-----------------------------
The code of [https://github.com/WordPress/wordpress-
develop/blob/24a70204db8d9ca5c6ddfc2c22247418fa0af5a1/src/wp-
includes/functions.php#L642 `is_serialized()`] is frozen
[https://core.trac.wordpress.org/ticket/17375#comment:37 since 2016] as a
security measure to protect WordPress and its plugins against PHP Object
Injection vulnerabilities, linking to the
[https://www.owasp.org/index.php/PHP_Object_Injection OWASP website] as a
reference.
The initial idea was that the `is_serialized()` function should return
`true` only for safe serialized strings, and `false` for unsafe ones to
prevent their deserialization by the caller.
However, the current implementation completely fails to achieve that and
does not bring any protection at all against the aforementioned threat.
Not only it doesn't add any security, but the fact that the code has been
flagged as "frozen in time" is nefarious as it blocks any further
developments (both [https://core.trac.wordpress.org/ticket/53299
functionally] and [https://core.trac.wordpress.org/ticket/53295 security]
wise).
== Standard objects ==
Let's take all three examples from the [https://owasp.org/www-
community/vulnerabilities/PHP_Object_Injection#examples OWASP page] used
to support and document this choice.
The input string being malicious, `is_serialized()` is expected to return
`false` for each of the following examples, as per documented in the
original [https://core.trac.wordpress.org/ticket/17375#comment:37 ticket
17375].
**''I highlight that these are the exact examples used to support and
document the choice of freezing `is_serialized()`''**, nothing has been
changed.
=== Example 1 ===
Malicious payload:
{{{
http://testsite.com/vuln.php?data=O:8:"Example1":1:{s:10:"cache_file";s:15:"../../index.php";}
}}}
Returned value:
{{{
var_dump(is_serialized('O:8:"Example1":1:{s:10:"cache_file";s:15:"../../index.php";}'));
# Output: bool(true)
}}}
The function **failed** to properly detect and block the malicious
serialized object.
=== Example 2 ===
Malicious payload:
{{{
Cookie:
data=O%3A8%3A%22Example2%22%3A1%3A%7Bs%3A14%3A%22%00Example2%00hook%22%3Bs%3A10%3A%22phpinfo%28%29%3B%22%3B%7D
}}}
Returned value:
{{{
var_dump(is_serialized(urldecode('O%3A8%3A%22Example2%22%3A1%3A%7Bs%3A14%3A%22%00Example2%00hook%22%3Bs%3A10%3A%22phpinfo%28%29%3B%22%3B%7D')));
# Output: bool(true)
}}}
The function **failed** to properly detect and block the malicious
serialized object.
=== Example 3 ===
Malicious payload generation:
{{{
class SQL_Row_Value
{
private $_table = "SQL Injection";
}
class Example3
{
protected $obj;
function __construct()
{
$this->obj = new SQL_Row_Value;
}
}
print urlencode(serialize(new Example3));
# Output:
O%3A8%3A%22Example3%22%3A1%3A%7Bs%3A6%3A%22%00%2A%00obj%22%3BO%3A13%3A%22SQL_Row_Value%22%3A1%3A%7Bs%3A21%3A%22%00SQL_Row_Value%00_table%22%3Bs%3A13%3A%22SQL+Injection%22%3B%7D%7D
}}}
Returned value:
{{{
var_dump(is_serialized(urldecode('O%3A8%3A%22Example3%22%3A1%3A%7Bs%3A6%3A%22%00%2A%00obj%22%3BO%3A13%3A%22SQL_Row_Value%22%3A1%3A%7Bs%3A21%3A%22%00SQL_Row_Value%00_table%22%
3Bs%3A13%3A%22SQL+Injection%22%3B%7D%7D')));
# Output: bool(true)
}}}
The function **failed** to properly detect and block the malicious
serialized object.
== Serializable classes ==
In addition to the standard serialized objects seen above, PHP also offers
[https://www.php.net/manual/en/class.serializable.php an alternate
mechanism] which basically replaces the call to the magic function
`_wakeup()` by a call to the Serializable interface function
`deserialize()`.
A specific attention has been given to prevent the deserialization of such
objects, notably by the addition of a [https://github.com/WordPress
/wordpress-
develop/blob/376840b83cb11167d5fe545e929b37d41ce58d6e/tests/phpunit/tests/functions.php#L367
dedicated test case]. In particular, `is_serialized()` ensures that the
first character of the serialized string must not be a 'C'.
Nevertheless, here again, if we take all OWASP examples and replace the
standard objects by a Serializable object embedded within an array,
`is_serialized()` systematically fails to detect and block the malicious
payload:
- Example 1:
{{{
var_dump(is_serialized('a:1:{i:0;C:8:"Example1":23:{s:15:"../../index.php";}}'));
# Output: bool(true)
}}}
- Example 2:
{{{
var_dump(is_serialized(urldecode('a%3A1%3A%7Bi%3A0%3BC%3A8%3A%22Example2%22%3A18%3A%7Bs%3A10%3A%22phpinfo%28%29%3B%22%3B%7D%7D')));
# Output: bool(true)
}}}
- Example 3:
{{{
var_dump(is_serialized(urldecode('a%3A1%3A%7Bi%3A0%3BC%3A8%3A%22Example3%22%3A72%3A%7BO%3A13%3A%22SQL_Row_Value%22%3A1%3A%7Bs%3A21%3A%22SQL_Row_Value_table%22%3Bs%3A13%3A%22S
QL+Injection%22%3B%7D%7D%7D')));
# Output: bool(true)
}}}
== Conclusion ==
Freezing `is_serialized()` code as a security measure to block malicious
payloads proves to be a totally ineffective measure as it fails to block
any single one of the examples linked when this decision was taken and did
not prevent WordPress from being vulnerable to any Insecure
Deserialization vulnerabilities since then (a very quick search shows
CVE-2020-36326, CVE-2020-28032, CVE-2019-12240, CVE-2018-20148, etc.).
This freeze should therefore be lifted to allow further developments,
notably the implementation of more efficient protections against such
threats.
--
Ticket URL: <https://core.trac.wordpress.org/ticket/53391>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list