fix(snapshots): unserialize restore payloads without instantiating classes#56
Conversation
…asses The rollback engine reads its snapshot payloads back with unserialize() on the restore path. The payload files are written by the plugin, but they live on disk in wp-content and are the one tamperable input restore() consumes — a plain unserialize() there lets a crafted payload construct arbitrary objects and fire __wakeup()/__destruct() gadget chains (PHP object injection). WPCS flags all three sites; this is the real one of the pair (serialize() on the write path is not exploitable and stays a visible warning). All three restore sites now go through a single unserialize_payload() helper that passes `allowed_classes => false`, so a serialized object is stripped to a __PHP_Incomplete_Class instead of being built: - option restore gained an explicit is_stripped_object() guard (it had none) and fails closed instead of writing the incomplete class back via update_option(). - meta / posts restore already required an array and now reject the stripped object through that same is_array() guard as "payload corrupt". No functional regression: everything the engine captures is scalars and arrays (option values, the meta/post maps it builds, Elementor's JSON-string payloads), which round-trip identically under allowed_classes => false — covered by a new scalar+nested-array round-trip test and the existing suite. Regression tests pin the security property directly: a gadget class whose __wakeup() flips a static flag is serialized into each payload kind; after restore the flag stays false and the restore fails closed. Mutation-checked — reverting the helper to a bare unserialize() makes the gadget test fail (the flag flips), so the test genuinely exercises the vulnerability.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de4dfa5594
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( $this->is_stripped_object( $value ) ) { | ||
| return array( 'success' => false, 'error' => 'Snapshot payload contains an object and cannot be safely restored.' ); | ||
| } | ||
| update_option( $record['target'], $value ); |
There was a problem hiding this comment.
When a tampered option payload is an array that contains a serialized object, allowed_classes => false produces a nested __PHP_Incomplete_Class, but this top-level check passes and update_option() stores the array as a successful restore. PHP serializes an incomplete object back using the original class name, so WordPress will later instantiate that object when the option is read/unserialized, re-opening the object-injection path this change is meant to close; the same recursive check is needed before writing meta/post payload contents too.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de4dfa5594
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( $this->is_stripped_object( $value ) ) { | ||
| return array( 'success' => false, 'error' => 'Snapshot payload contains an object and cannot be safely restored.' ); | ||
| } | ||
| update_option( $record['target'], $value ); |
There was a problem hiding this comment.
When the tampered payload is an array that contains a serialized object, allowed_classes => false still produces a nested __PHP_Incomplete_Class, but this top-level check does not catch it and update_option() stores the array while reporting a successful restore; the same pattern also lets meta/posts array payloads flow into restore_meta_map(). Since snapshot payloads are treated as tamperable input and the new behavior is intended to fail closed on objects, the restored value should be walked recursively and rejected if any incomplete class is present before writing it back.
Useful? React with 👍 / 👎.
… round-1 P1) The first pass only checked the TOP level of a restored payload for a stripped object. A tampered payload can nest one inside an otherwise-valid array — serialize( array( 'x' => $gadget ) ) for an option, or a gadget in a captured meta value — and allowed_classes => false turns it into a nested __PHP_Incomplete_Class. The top-level check saw an array, passed, and update_option()/update_post_meta() persisted the incomplete class while reporting a successful restore. is_stripped_object() is replaced by contains_stripped_object(), which walks the whole structure. All three restore kinds now reject a stripped object at any depth: option fails closed on it, meta/posts fold it into their existing "payload corrupt" guard. Regression tests cover the nested case for option and meta (a gadget hidden in an array leaf). Mutation-checked: collapsing the walk back to a top-level-only check makes both new tests fail (restore wrongly succeeds), so they exercise the P1 rather than passing vacuously. Full suite green (510 tests).
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 985050894c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| if ( is_array( $value ) ) { | ||
| foreach ( $value as $item ) { | ||
| if ( $this->contains_stripped_object( $item ) ) { |
There was a problem hiding this comment.
When the restore payload is tampered to contain a valid serialized array with references/cycles (for example a self-referential array), this recursive walk follows the cycle indefinitely and the REST restore request fatals from stack/memory exhaustion instead of failing closed. Since the payload file is explicitly treated as tamperable input, the scanner needs cycle/depth protection or should reject reference-heavy structures before recursing.
Useful? React with 👍 / 👎.
…es (Codex round-2 P2)
The recursive contains_stripped_object() added in the previous commit had no
cycle guard. unserialize() faithfully rebuilds a serialized reference cycle —
e.g. `a:2:{s:1:"k";i:1;s:4:"self";R:1;}` — into a genuinely self-referential
array (verified: allowed_classes => false does not stop this, references are not
objects). Walking one unbounded exhausts the stack and fatals the REST restore
request instead of failing closed, so a tampered payload turns a rejection into
a DoS.
The walk is now depth-bounded (MAX_WALK_DEPTH = 64) and treats hitting the cap
as unsafe — return true, i.e. reject the payload. Real snapshot payloads (option
values, the shallow meta/post maps, Elementor's JSON-string blobs) are nowhere
near 64 levels deep, so nothing legitimate is affected; the cap only bounds a
crafted one.
Regression test feeds a self-referential payload and asserts the restore fails
closed, returns quickly (< 1s, i.e. terminates rather than spinning), and leaves
the live option untouched. Full suite green (511 tests).
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…es (#57) Cuts a release for the security hardening that landed since 2.8.1. The driver is the object-injection defense on the snapshot/rollback restore path (#56): stored payloads are now unserialized with allowed_classes => false and restores fail closed on any object-bearing, nested-object, cyclic, or malformed payload. Also ships the Plugin Check correctness fixes from #54 (wp_delete_file in the self-updater, sitemap state read via the sitemaps server). Bumps the plugin header, AURA_WORKER_VERSION, and the readme Stable tag to 2.8.2, and adds the changelog + a <300-char upgrade notice. The CI gate added in #55 is noted as internal — it does not ship in the plugin zip.
1 participant
The bug
The snapshot/rollback engine reads its payloads back with
unserialize()on the restore path (3 sites inclass-aura-worker-snapshots.php). The payload files are written by the plugin, but they live on disk underwp-contentand are the one tamperable inputrestore()consumes. A plainunserialize()there lets a crafted payload construct arbitrary objects and fire__wakeup()/__destruct()gadget chains — classic PHP object injection.Surfaced as a warning by the new PHPCS gate (#55). This is the exploitable half of the pair;
serialize()on the write path is not a vulnerability and stays a visible warning.The fix
All three restore sites now route through one
unserialize_payload()helper that passesallowed_classes => false, so a serialized object is stripped to a__PHP_Incomplete_Classinstead of being built:unserialize($raw)straight intoupdate_option(). Added an explicitis_stripped_object()check; it now fails closed rather than persisting an incomplete class.is_array($captured), so a stripped object falls through that existing guard as "payload corrupt".Why no functional regression
Everything the engine actually captures is scalars and arrays — option values, the meta/post maps it builds, Elementor's JSON-string payloads. All of those round-trip identically under
allowed_classes => false. Covered by a new scalar + nested-array round-trip test plus the existing suite (the pathological object-valued-option case is astronomically rare in an Elementor-rollback tool, and failing closed there beats object injection).Tests pin the security property — and are mutation-checked
A gadget class whose
__wakeup()flips a static flag is serialized into each payload kind (option / meta / posts); after restore the flag staysfalseand the restore fails closed.Mutation check: reverting the helper to a bare
unserialize()makes the gadget test fail (the flag flips), proving the test genuinely exercises the vulnerability rather than passing vacuously.Verification
php -lclean; PHPCS gate exits 0; no rawunserialize(remains outside the helper.🤖 Generated with Claude Code