Tag Archives: PHP

Improving Plugin Security One Day at a Time

As a security-conscious developer, I like to inspect a WordPress plugin’s source code before I install it on one of my sites. I didn’t do this, formerly. But after the first time I did (and found a vulnerability), I believe more strongly in its importance.

I also keep tabs on WordPress related security reports on sites like Packet Storm, Secunia, Exploit Database, and Bugtraq. I use IFTTT for this:

IFTTT Recipe: Email me WordPress security reports from Packet Storm connects feed to email

IFTTT Recipe: Email me WordPress security reports on Secunia connects feed to email

IFTTT Recipe: Email me WordPress security reports on Exploit DB connects feed to email

IFTTT Recipe: Email me WordPress security reports from Bugtraq connects feed to email

Over the last year I’ve come to realize something: almost every plugin has vulnerabilities. Okay, all code has vulnerabilities. But almost every plugin has glaringly obvious vulnerabilities, or at least that can be found without a great deal of effort.

That seems a little bit scary. Of course, many of these aren’t particularly serious. But they demonstrate that the people creating the plugins often don’t understand basic WordPress security.

I know I’m not the only one who realizes this. Probably, most experienced WordPress developers come to this realization sooner or later. But just realizing it and thinking, “I wish it weren’t so,” isn’t going to make things any better. So I’ve decided to do something about it. I’m going to improve plugin security one day at a time. I’m going to try to do something, every day, that will make WordPress plugins more secure.

How will I do this? In many ways. I said that I’m not the only one who understands the situation, and I’m not the only one who’s doing things about it either. The folks on the plugin review team for WordPress.org try to catch the vulnerabilities when a plugin is first submitted to the repo. They also handle security reports and make sure they make it to the plugin authors. There are also the great folks on the WordPress docs team contributing to the plugin developer handbook. Hopefully the security-related things in there will help to educate the next generation of plugin developers about these issues better right from the start.

One of the greatest things I can do is to try to help educate plugin devs about security. I’ll also try to make sure that reports of vulnerabilities make their way to the plugin developer. And I’ll continue to review plugins that I use, and report vulnerabilities that I find. I might then move on to investigating other popular plugins as well. I have even pondered creating a PHP source code security scanner, but that would be quite a project. (There are many of these out there, but none of them are intelligent enough for me.)

Regardless of how, I want to try to do a little something every day to improve WordPress plugin security. If just a few folks did this, how different might things look in a few years? We’ll just have to wait and see.

Creating Your Own WordPress Unit Test Factories

WordPress has these things in its PHPUnit test library called factories. Their purpose is to allow you to easily create things, like posts.

You might wonder why that’d be so helpful, since after all, WordPress already provides functions like wp_insert_post(). If you are wondering that, maybe you haven’t written very many unit tests.

The problem with wp_instert_post() et al. is that you have to make up a lot of the post’s attributes, like its title and content. While this can be amusing, it can quickly become boring and time consuming. This is especially so when those fields don’t matter in your test in the first place.

WordPress’s solution to this is to provide these factories in its test cases. When your test case extends WP_UnitTestCase, you have access to the factory property, which is a WP_UnitTest_Factory instance. The factory itself has several properties, like post, which is a WP_UnitTest_Factory_For_Post instance.

So you can create a post just by calling $this->factory->post->create(). You don’t have to worry about the post’s attributes, because they will be generated as needed. And if you do need to set the title, for example, you can easily do that:

$this->factory->post->create( array( 'post_title' => 'My Title' ) );

There are other factories as well, for users, attachments, comments, etc. They pretty much cover everything you’d want a factory for in WordPress.

But sometimes a plugin has its own entities that it needs to create in its unit tests. WooCommerce orders, for example. This can be achieved by creating custom factories. You just need to create your own child of WP_UnitTest_Factory_For_Thing, which all of the factories extend. It has just three abstract methods that you’ll need to create: create_object, update_object, and get_object_by_id. It’s pretty simple to implement these, and they do exactly what you’d expect based on their names.

__construct()

Oh, did I forget to mention the constructor? That is actually one of the most important parts. In your constructor is where you have the opportunity to set up the default values for each of the entities’ properties. For example, in the post factory, the constructor looks like this:

	function __construct( $factory = null ) {
		parent::__construct( $factory );
		$this->default_generation_definitions = array(
			'post_status' => 'publish',
			'post_title' => new WP_UnitTest_Generator_Sequence( 'Post title %s' ),
			'post_content' => new WP_UnitTest_Generator_Sequence( 'Post content %s' ),
			'post_excerpt' => new WP_UnitTest_Generator_Sequence( 'Post excerpt %s' ),
			'post_type' => 'post'
		);
	}

The post status and post type default to scalar values, so that shouldn’t be unfamiliar to you. The really interesting part here is the WP_UnitTest_Generator_Sequences. As you can see, these are constructed with a string that contains a %s placeholder. The string will be used as the content for the default created posts, but the placeholder will be replaced with an integer. That number is from an iterator in the generator that gets incremented each time the field needs to be generated. So the first post title generated will be ‘Post title 1’ and the second will be ‘Post title 2’. This means that the generated fields will be unique, which can be especially good when debugging.

create_object()

The create_object() method is called by the higher-level methods create(), create_and_get(), and create_many(). It is passed an array of arguments, and is expected to return the ID of the object that’s created, or false or a WP_Error if it fails. In the post factory, it looks like this:

	function create_object( $args ) {
		return wp_insert_post( $args );
	}

That’s very simple, isn’t it? The $args have already been merged with the defaults we defined in the constructor before they get passed in, so all we need to do is insert the post.

update_object()

The update_object() method is called by the create() method, strangely enough. It is used to update the object’s fields after applying any callbacks that have been registered (that’s another story for another time). It gets passed the ID of the object to update, and an array of fields to update. The return value should again be false or a WP_Error object.

In the post factory it looks like this:

	function update_object( $post_id, $fields ) {
		$fields['ID'] = $post_id;
		return wp_update_post( $fields );
	}

Again, really simple. We just set the ID as one of the fields, since that’s the way wp_update_post() takes its arguments.

get_object_by_id()

The get_object_by_id() method is called by create_and_get() to retrieve the object once it has been created. It is passed an ID, and is expected to return that object.

In the post factory, it looks like this:

	function get_object_by_id( $post_id ) {
		return get_post( $post_id );
	}

There’s not much to see here either. Just retrieve the post and return it.

Conclusion

All in one piece, the post factory looks like this:

class WP_UnitTest_Factory_For_Post extends WP_UnitTest_Factory_For_Thing {

	function __construct( $factory = null ) {
		parent::__construct( $factory );
		$this->default_generation_definitions = array(
			'post_status' => 'publish',
			'post_title' => new WP_UnitTest_Generator_Sequence( 'Post title %s' ),
			'post_content' => new WP_UnitTest_Generator_Sequence( 'Post content %s' ),
			'post_excerpt' => new WP_UnitTest_Generator_Sequence( 'Post excerpt %s' ),
			'post_type' => 'post'
		);
	}

	function create_object( $args ) {
		return wp_insert_post( $args );
	}

	function update_object( $post_id, $fields ) {
		$fields['ID'] = $post_id;
		return wp_update_post( $fields );
	}

	function get_object_by_id( $post_id ) {
		return get_post( $post_id );
	}
}

You might be amazed that this little bit of code could really be so helpful, but I assure you that it is. It is fully worth creating factories for custom objects if you need to use them much in your tests. They may not all be as simple as this one, but that is only another reason to build it and keep your code DRY.

WordPress Unit Testing with WP_Http

If you use wp_remote_request() or other wrappers for WP_Http methods in your code, this makes it difficult to test, especially if the remote server may not be reachable from your testing environment. That’s why I created the WP_HTTP_UnitTestCase to solve this by letting you mock the remote responses.

The usage details are there in the repository’s readme file.

It lets you mock the remote responses coming from the server, and also captures the data that was sent to the remote server so you can check that the correct requests are made.

As I continue to write unit tests for WordPress plugins I create, I think I’ll find this helper useful. I hope you might as well.

WordPress Core and PHP version support

WordPress core has supported PHP versions >= 5.2.17 for a long time. And there’s been many a discussion by developers and contributors about dropping support for 5.2, making the minimum requirement 5.3. But the lead developers are standing against this move as long as 5.2 makes up a significant part of the user base. And less than a year ago it made up about 50%.

We know the approximate proportions of which versions are being used because this information is available through the stats page on wordpress.org. I’ve been watching this page for a while, and I’ve caught a definite trend away from PHP 5.2 toward newer versions, especially 5.3.

Today, David Anderson posted this to the wp-hackers mailing list:

Rejoice: PHP 5.3 is, at last, on the threshold of overtaking PHP 5.2 as the most common version that WordPress is being run on…

Stats

… just in time for the PHP 5.3 end-of-life! http://marc.info/?l=php-internals&m=140605526629324&w=2 (though for many, there will continue to be security patches for several more years, since PHP 5.3 was part of RHEL 5 and hence Cent OS 5).

From the end of this month, around 21.5% of WordPress installations will be running on non-EOL-ed PHP versions.

Best wishes,
David

You can see this for yourself:

PHP versions WordPress is running on

If you want to know where my sites are on the graph, they make up a part of that green streak for 5.5. :-)

As you can see, this is a mixture of good and bad news. The good news is that people are moving away from 5.2, which no longer recieves security patches from PHP. The bad news is that the majority of these folks, who are moving to 5.3, will just be back in the same boat at the end of the month. And so will WordPress.

It’s not that WordPress needs to drop support for 5.2 so that it can introduce cool new features or improve its performance. Sure, there may be a few ways in which 5.2 is holding us back, but they really aren’t that significant. It’s mostly that the grass is always greener on the other side of the fence.

The thing that really strikes me here is how openly vulnerable software is dominating the market. It isn’t just the PHP versions WordPress is running on, it’s WordPress itself:

Versions of WordPress Pie Chart

What has always seemed strange to me is that only a fifth of sites are running the latest major version of WordPress (3.9). It makes me wonder what the breakdown of PHP versions is relative to WordPress versions, but if I remember correctly, we’ve been told that it’s pretty much the same as overall.

The maintained WordPress versions (3.7, 3.8, 3.9), make up only about 38% of the live websites it’s running on out there. In other words, 62% of WordPress sites are vulnerable right off the bat. And, what really strikes me, is that WordPress 3.0 continues to maintain a larger portion of the market than any other version.

Why is this? Why do folks continue to run old versions of WordPress? I assume it has much to do with a perceived difficulty in upgrading, whether that difficulty is real or imagined. I’m guessing people think of it like this:

  1. Oh, there’s a new version of WordPress.
  2. Updating takes time though, and what if something goes wrong?
  3. I hear there are new features, but do I really need them?
  4. Nah, my site is running fine, it isn’t worth the trouble.

Basically,

updates = (new features) - time - (the chance your site will break)

It seems that many people think that

new features < time + (the chance your site will break)

And in that case, it really doesn’t make sense to update.

But, of course, the equation is wrong. The truth is that

updates = (new features) + (bug fixes) + (security patches) - time - (the chance your site will break)

It’s those security patches that push updates from the trivial to the must.

That’s why I wrote a post asking plugin developers not to add backward compatibility code to support older WordPress versions. Basically, nobody should ever be running those versions.

Which brings us back around to the unmaintained PHP versions that most sites are running on. Should WordPress continue to support these versions? You might think I’d have to say “No”. But in this case, it’s a little more complicated. People know they are running WordPress (at least most of them do), and they either know or could easily find out what version of it they are running. But PHP, well, most people don’t know what on earth that is. And even those that do often have no control over what version they are running. In other words, it is up to the web host.

That doesn’t mean WordPress couldn’t drop an older version of PHP. But it means that if it did, people who use WordPress would panic, because they don’t really understand what is happening, why it is happening. And most of them have no control over what version of PHP they are running. So, you’d have to tell them to contact their host. But their host may or may not be able to just provide a newer version of PHP with a snap of the fingers. In other words, dropping a PHP version that 40% of sites are running on would probably alienate a lot of users, who are using WordPress precisely because they don’t have to worry about all of that technical stuff. But does that mean that we have to put up with hosts running dead PHP versions “for the sake of our users”, when that is possibly making those same users’ sites more vulnerable?

Let’s go back to what I said about WordPress’ lead developers reluctance to drop support for 5.2:

It’s not that WordPress needs to drop support for 5.2 so that it can introduce cool new features or improve its performance. Sure, there may be a few ways in which 5.2 is holding us back, but they really aren’t that significant. It’s mostly that the grass is always greener on the other side of the fence.

In other words, it’s the same false argument being used by folks who don’t feel like updating WordPress: I don’t really need the features, so it isn’t worth the trouble.

Of course, there is a difference. WordPress isn’t responsible for whether a user’s hosting is secure, the host is. And, as David Anderson pointed out, many OS’s and hosts will be providing their own security patches for PHP. In other words, the equation is much harder to solve than it looks. Offering security updates for WordPress, and hoping hosts patch their PHP, may indeed be the best course of action.

One time the unit tests failed me

Since my initiation into the world of unit testing about a year or so ago, I’ve been a huge fan of unit testing. I write unit tests for almost everything now, rather than doing repeated manual testing, which is sometimes more tedious (and certainly gets boring after a while).

But just now, the unit tests failed me. I’ll explain the case in a moment, but let me just say that, as great as unit tests are, they aren’t the only kind of testing you need. Doing manual beta testing is always needed for good measure.

So here’s what happened. I had written a custom module for my WordPoints plugin, with unit tests, of course. And the unit tests were passing fine.

That was last week. But before I tried it out on a live site, I wanted to test it manually on one of my local setups. And guess what. It didn’t work.

I eventually figured out that the issue was a bug in WordPoints, that was caused by WordPress having a 64 character limit for option names. I knew about this limit, but it hadn’t bitten me yet. I’d actually forgotten whether I’d handled the limit properly or not. But that is by the by.

What struck me after I figured out what was going on was this: “What about the unit tests I wrote, they passed fine!”

Yep, they did. And here’s why. For the unit tests, WordPress’s testcase sets up a database transaction before each test. Of course it doesn’t actually commit the data to the database, instead it rolls back the transaction after the test is done. This way, the database stays clean between tests, so they don’t leak into each other.

And this is why my tests didn’t fail. The data wasn’t actually getting committed to the database, so saving the option value wasn’t failing like it normally would. I assume that the database checks that the table and column names are all good, but doesn’t realize that you’re trying to write 65 characters to a VARCHAR(64) field until the data gets committed (which in this case it doesn’t).

In the end, I really think the issue here is that WordPress should fail, or tell us that we’re _doing_it_wrong(), when we try to use option names that are too long. But until then, we need to be careful, when using dynamic option names, that we check they aren’t too long. Especially since unit tests won’t be enough to catch the issue.