WordPress Documentation Doesn’t Warn About Security Risk of maybe_unserialize()
Last week we looked at an insecure WordPress function, maybe_unserialize() that was part of the cause of a “critical” vulnerability that was receiving press coverage. We noted a couple of troubling conversations on the Trac ticket system for WordPress related to that function and PHP object injection, which the insecure function permits. A commenter on the post noted another relevant Trac conversation that raises more concerns.
Someone labeled as a Core Committer of WordPress in part wrote this in 2017:
Most calls to
maybe_unserialize()
are done internally by WP (e.g., insideget_option()
), rather than directly by plugins, so I’m not sure how plugins or site admins would be able to take advantage of the extra parameter setup in ticket-37757.2.patch, unless they started bypassing the API and calling it directly, which I don’t think we want to encourage.
In you look at the WordPress documentation for maybe_unserialize() it doesn’t say anything about plugins not calling the function directly. That person is an employee of Automattic. Among the many plugins that call function directly, are two multi-million install plugins from Automattic. Two multi-million install security plugins also call it directly.
That person then wrote this:
I think it’d also be good to keep in mind that `unserialize()` is considered dangerous even with `allowed_classes`, so if something like this is merged, it might be good to make it very clear in the filter docblock that it shouldn’t be considered a safe way to use
unserialize()
, and that there’s no guarantee it’ll prevent vulnerabilities; it’s just extra hardening, andunserialize()
should still be avoided as much as possible. It’s fine to usemaybe_unserialize()
indirectly through the API, since Core keeps it safe, but inputs should still be validated, and if a plugin needs to directly encode non-scalar data, it should use JSON.
The change that person is referencing would prevent the insecurity that already exists in maybe_unserialize(), so adding a warning when including it is an odd choice. There should already have been a warning, but there still isn’t even now.
The vulnerability that led to our previous post didn’t involve a direct call to maybe_unserialize(), so even indirect calls to can be insecure.
WordPress Security Team Badly Needed
This situation again highlights the need for WordPress to finally get a security team in place. The comments we highlighted in this post and the past one show there is a clear disconnect between practical handling of security and what is going on with the development of WordPress. There seems to be a lack of understanding of the security implications of the security risks posed by security issues and how plugin developers are implementing WordPress functionality in the real world. This isn’t a new issue, the lack of response to the is_admin() issue has been noted for years.
You might be saying that WordPress already has a security team. In name they do. But even basic information on the team is missing. What is in their remit? Who is on the team? Who runs the team? What are the criteria for being on the team? Looking at the blog for the team, it doesn’t appear they do much. In recent years there have only been a few posts, one about older versions of WordPress losing support and the others about bug bounties. And one of the few posters on that is none other than the Core Contributor quoted above.
Checking For Usage of maybe_unserialize() With Our Plugin Security Scorecard
Our new Plugin Security Scorecard is intended to help incentivize WordPress plugin developers to implement better security practices. As we noted in our previous post, in line with that, we decided to add to the grading a check to see if the plugin is using maybe_unserialize() until WordPress implements security to it to avoid PHP object injection. WordPress really should fix this, but that it hasn’t been fixed so far, doesn’t suggest a fix is coming anytime soon. Therefore, plugin developers should take action to make sure there isn’t a chance of a vulnerability caused by this (they also could put pressure on WordPress to fix this).
That seems like an even better call when a (current?) member of the WordPress security team thinks that plugin developers shouldn’t be calling it directly.