Version 4.0.0 of an old tour-packages plugin went out at half past ten on a Wednesday. Version 4.0.5 went out at seven minutes past twelve.
None of those five patches were planned. Each one fixed something, and each fix took away a thing that had been hiding the next problem. By the end of the morning I’d learnt that a fatal error on a public search page had, for years, been the only thing standing between visitors and a reflected XSS hole. That’s not a sentence I enjoy typing.
Some background. The plugin is old. It registers a custom post type for travel packages, adds a couple of custom routes (/package/<slug> for detail pages and /packagesearch/ for results), and has been quietly running on single installs and one network-activated multisite since well before PHP 8. The 4.0.0 release was meant to be modest: bring the enquiry forms up to date with the current major version of Gravity Forms and drop an add-on that had been abandoned. Bump the major, ship it, go and get a coffee.
4.0.1: plugins load before themes
4.0.1 went out seventeen minutes later. Sites on the multisite running one particular child theme were white-screening with:
PHP Fatal error: Cannot redeclare example_add_qs_var()
The plugin carries a small helper for adding a key to a querystring. Years ago the same helper got copied into a child theme, and the plugin’s copy was wrapped in the usual guard:
if ( ! function_exists( 'example_add_qs_var' ) ) {
function example_add_qs_var( $url, $key, $value ) {
// ...
}
}
That guard looks like it handles the collision. It only handles it in one direction. WordPress loads plugins before the theme, so the plugin checks, finds nothing, declares the function, and then the theme declares it again without a guard. Fatal.
Why now and not in the five years that helper has existed? I don’t have a satisfying answer. The function hadn’t changed. My best guess is that this release was simply the first in a long while to reach the sites running that theme, and nobody had ever had both copies loaded at once. Not proud of the guess, but I’d rather say so than make one up.
The fix is to declare the plugin’s fallback after the theme has had its turn:
add_action( 'after_setup_theme', function () {
if ( function_exists( 'example_add_qs_var' ) ) {
return;
}
function example_add_qs_var( $url, $key, $value ) {
// ...
}
} );
The theme’s copy wins if it has one, and sites without one still get the plugin’s. A function_exists() guard only protects you from whoever loads after you.
4.0.2: null into a constructor
With the sites back up, the next thing was 500s on /package/ URLs that didn’t exist. Deleted packages, old links, crawlers. The sort of request every site gets all day.
The route handler did roughly this:
$post = get_page_by_path( $slug, OBJECT, 'example_package' ); $wp_query->queried_object = new WP_Post( $post );
For a slug that doesn’t exist, get_page_by_path() returns null, and on PHP 8 new WP_Post( null ) is a TypeError. Every one of those requests should have been a plain 404.
if ( ! $post ) {
$wp_query->set_404();
status_header( 404 );
nocache_headers();
return;
}
Fine. Small. Moving on.
4.0.3: the flush that was covering for the activation hook
While I was in the route class I noticed this in the init callback:
add_rewrite_rule( '^package/([^/]*)/?', 'index.php?package=$matches[1]', 'top' ); global $wp_rewrite; $wp_rewrite->flush_rules( false );
Flushing rewrite rules on every request. Regenerating the full rule set and writing it back to the options table, on every page load, on every site. There’s even a comment from the original tutorial it was copied from, a few lines up in the activation code, telling you in capitals never to do exactly this.
Taking it out was the easy half. The interesting half was what it had been hiding.
The plugin also flushes on activation, like you’re supposed to. But when WordPress activates a plugin it loads that plugin after init has already fired, so the post type, the taxonomies and the two custom routes hadn’t been registered yet when the activation flush ran. It saved a rule set that didn’t contain any of them. Nobody ever noticed, because the very next page load flushed again with everything registered.
Remove the per-request flush and reactivating the plugin would leave every package page 404ing. So the activation hook now registers everything itself before flushing:
function example_packages_activate() {
if ( ! post_type_exists( 'example_package' ) ) {
example_register_post_type();
}
Example_Package_Detail::add_rewrite_rules();
Example_Package_Search::add_rewrite_rules();
flush_rewrite_rules();
}
And existing sites, which won’t run the activation hook again, get one flush from a version-upgrade step. That one has its own small trap. The upgrade check runs on plugins_loaded, where $wp_rewrite doesn’t exist yet, so flush_rewrite_rules() there silently does nothing. It has to be deferred:
if ( version_compare( $version, '4.0.3', '<' ) ) {
add_action( 'wp_loaded', function () {
flush_rewrite_rules( false );
} );
$version = '4.0.3';
}
wp_loaded comes after init, so the routes are registered by then and the flush actually saves them.
4.0.4: drafts are public if you know the URL
Six minutes later. Looking at that same get_page_by_path() call I realised it doesn’t care about post status. At all. Draft, pending, scheduled, private, or a package that was live last month and then pulled: if you had the URL, the route rendered it.
It also falls back to attachments, so /package/some-image-slug/ rendered an image attachment inside the package template. And an empty ?package= matched packages that didn’t have a slug yet, of which the multisite had a few hundred sitting in drafts. And ?package[]=x was another fatal, because the function wants a string.
$slug = $wp_query->query_vars['package'];
$post = is_string( $slug ) && '' !== $slug
? get_page_by_path( $slug, OBJECT, 'example_package' )
: null;
if ( ! $post
|| 'example_package' !== $post->post_type
|| ! ( is_post_publicly_viewable( $post ) || current_user_can( 'read_post', $post->ID ) ) ) {
$wp_query->set_404();
status_header( 404 );
nocache_headers();
return;
}
The current_user_can( 'read_post' ) part keeps previews working for editors. On a multisite that’s per-site, so an editor on one site can’t preview another site’s drafts, which is what you want.
I think this is the one that would have hurt in real life. A travel package that gets pulled because the supplier dropped a price or withdrew a departure is exactly the kind of page somebody has bookmarked.
4.0.5: the crash that was doing security work
Last one. The search results page (/packagesearch/) was a hard 500 on every site that wasn’t using one particular commercial theme, and on current versions of that theme too, because the template called a layout method the theme no longer has. I honestly don’t know how long package search had been dead on most sites. Long enough that nobody had complained loudly enough for it to reach me, which says something about package search, I suppose.
Fixing the crash is one line. But before shipping that line I read the rest of the template, because a page that’s been dead for ages hasn’t had anyone look at it either. Good thing.
The search terms went straight from $_POST back onto the page:
<p>Search terms: <?php echo join( ' - ', $terms ); ?></p>
where $terms included the raw tour code someone had typed. Reflected XSS.
And the results were produced by gluing the search input into a shortcode string and running it:
$params .= ' package_code="' . $_POST['packageCode'] . '"'; echo do_shortcode( '[example_packages' . $params . ']' );
Close the quote and the bracket in your search box and you can open any shortcode the site has registered, with whatever attributes you like. Some shortcodes are harmless. Some aren’t.
Neither of these was reachable while the page fataled before it got that far. The moment I fixed the crash I’d have published both. So the fix went out as one release:
$atts = array();
foreach ( $term_fields as $field => $att ) {
$raw = isset( $_POST[ $field ] ) && is_string( $_POST[ $field ] ) ? wp_unslash( $_POST[ $field ] ) : '';
$slug = sanitize_title( $raw );
if ( '' !== $slug ) {
$atts[ $att ] = $slug;
}
}
$code = sanitize_text_field( wp_unslash( $_POST['packageCode'] ?? '' ) );
if ( '' !== $code ) {
$atts['package_code'] = $code;
}
echo esc_html( join( ' - ', $labels ) );
echo example_packages_render( $atts ); // call the function, not do_shortcode()
The shortcode’s render function gets an array. No string is ever built, so there’s nothing to break out of. Taxonomy fields get reduced to slugs, which also fixed destination search, because the destination dropdown had been posting the term name all along and matching nothing. Another bug nobody saw, because the page never loaded.
The bit I keep thinking about
Every one of those five problems was there before 4.0.0. The release didn’t create them. It just started pulling on the thread, and each fix took away whatever had been making the next problem invisible: a theme that happened to load late, a flush that ran on every request, a page that crashed before it could do any damage.
The one that worries me is the last. If I’d fixed the crash on its own — a one-line, obviously correct, low-risk change — I’d have shipped an XSS and a shortcode injection, with a tidy commit message.
So now, when a page has been broken for a long time, I read the whole thing before I make it work again. It hasn’t been running, so nobody’s been watching what it does.