Start with the patch that didn't work. In GiveWP 4.16.6 someone sat down, understood the bug, wrote a recursive check that walks the unserialized result looking for __PHP_Incomplete_Class, and then, on finding one, returned the raw $data string. Detection: correct. Response: hand the attacker's bytes straight back to the caller. That is the sentence I want you to sit with, because it is the whole vulnerability in miniature. A sanitizing function that cannot bring itself to destroy data has not sanitized anything, it has just moved the explosion one stack frame further down. 4.16.7.2 returns false instead, and that one-word difference is the difference between a fix and a comment.

if (self::containsPhpIncompleteClass($unserializedData)) {
    // 4.16.6 returned $data here, which re-armed the payload
    return false;
}

How does an experienced team end up there? Because the PHP API invites it. safeUnserialize() in src/Helpers/Utils.php calls unserialize() with ['allowed_classes' => false], which reads like a kill switch and is documented as nothing of the sort. The manual is blunt: the option does not prevent object creation. You get a __PHP_Incomplete_Class placeholder that faithfully preserves the original class name and every single property. Read it and nothing happens. Serialize it again and PHP emits the identical byte sequence you started with. The flag protects the current read, and only the current read.

The path in this CVE is almost boring, which is why it's worth memorising. An attacker puts a serialized gadget payload in the last_name field of their own account through profile.php. On donation submit, includes/process-donation.php assembles user_info from that account data, runs every field through the helper, and writes the resulting array into the wp_give_sessions table. The next request reads that row and calls unserialize() with no guard at all, and the object wakes up for real. Nothing in the request body was ever malicious in a way a validator would notice. Your own database is user input with a nicer reputation.

The honest counter-argument is that returning false is destructive, and destructive changes generate support tickets. Somewhere out there is a donor whose company name genuinely trips containsSerializedDataRegex, and the 4.16.6 approach was, if you squint, conservative: preserve the user's data, don't lose a donation. I'd take the false anyway, every time. Weigh the two failure modes honestly. On one side, a session row gets dropped and a donor re-submits a form. On the other, unauthenticated system() as the web server user on a plugin with more than 100,000 installs, every version up to 4.16.7.1, CVSS 10.0. That is not a close call, and treating it as one is how you end up writing a check whose only effect is to prove you saw the problem.

What I actually admire about 4.16.7.2 is that it doesn't pretend to know which layer mattered. The write path now refuses the donation if a name field contains serialized data. Three separate read sinks got explicit allowed_classes guards: the session getter in class-give-session.php, the session table read in class-give-db-sessions.php, and the donor wall in class-give-donor-wall.php, which turned out to be reachable by an anonymous visitor through the public [give_donor_wall] shortcode without so much as a cookie. And ProviderForwarder::__call() now checks that the resolved provider implements the contract before it calls anything. Four independent breaks in one chain is not paranoia, it's an admission that the person fixing a deserialization bug cannot enumerate every sink. Good.

The gadget half deserves its own uncomfortable question. The chain runs through TCPDF, which the plugin bundles for PDF generation, into Give\TestData classes that exist to generate demo data. __call() looked up a name in loadedProviders, a plain array property with no type on it, and passed whatever it found to call_user_func_array(). Set that property to the string system during deserialization, let TCPDF::__destruct() do its thing, and you have a shell. I'm not going to pretend I audit my own vendor directory for this. But if seeding and test-fixture code ships to production alongside a PDF library that has magic methods in its destructor path, the two of them are colleagues now, whether you introduced them or not.

One more thing that people will get wrong in the next fortnight: the version bump is not the remediation. If a payload was planted before you updated, it is still sitting in your tables. That's why 4.16.7.2 ships a SanitizeSerializedObjectPayloads migration across usermeta, give_donormeta, give_donationmeta and give_sessions, replacing nested objects with empty strings. Check that it actually ran. And note that give_action=user_register still ignores the users_can_register option in 4.16.7.2, so accounts remain free for the taking on sites where registration is off. Patchstack files that as its own access-control problem now that the RCE chain is broken, which is fair, but it's your WAF rule to write.

So here's what I'd like to argue about in the comments. Do you unserialize anything at all that came out of your own database, and if you do, what happens to a row that fails the check? Dropped silently, dropped loudly into a dead-letter table, or flagged for a human? I've been on the fail-closed side of that meeting and lost it more than once, because someone reasonably asked how many legitimate records we'd shred. If you've found a version of this that operations people accept, I want to hear how you phrased it.