Skip to content

fix(snapshots): unserialize restore payloads without instantiating classes #56

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
BenKalsky merged 3 commits into from
Jul 15, 2026
Merged

fix(snapshots): unserialize restore payloads without instantiating classes #56

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
de4dfa5
fix(snapshots): unserialize restore payloads without instantiating cl…
BenKalsky Jul 15, 2026
9850508
fix(snapshots): reject stripped objects nested in payloads too (Codex…
BenKalsky Jul 15, 2026
15d0f4c
fix(snapshots): bound the stripped-object walk against reference cycl…
BenKalsky Jul 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 93 additions & 5 deletions digitizer-site-worker/includes/class-aura-worker-snapshots.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@

if ( null !== $payload ) {
$payload_path = $this->dir . $id . '.payload';
if ( false === file_put_contents( $payload_path, $payload ) ) {

Check warning on line 84 in digitizer-site-worker/includes/class-aura-worker-snapshots.php

View workflow job for this annotation

GitHub Actions / PHPCS 

File operations should use WP_Filesystem methods instead of direct PHP filesystem calls. Found: file_put_contents().
return false;
}
$meta['payload_path'] = $payload_path;
Expand All @@ -92,7 +92,7 @@
return false;
}
$meta_path = $this->dir . $id . '.json';
if ( false === file_put_contents( $meta_path, $json ) ) {

Check warning on line 95 in digitizer-site-worker/includes/class-aura-worker-snapshots.php

View workflow job for this annotation

GitHub Actions / PHPCS 

File operations should use WP_Filesystem methods instead of direct PHP filesystem calls. Found: file_put_contents().
return false;
}
$meta['meta_path'] = $meta_path;
Expand All @@ -111,7 +111,7 @@
return array( 'success' => false, 'error' => 'File not found: ' . (string) $path );
}

$contents = file_get_contents( $path );

Check warning on line 114 in digitizer-site-worker/includes/class-aura-worker-snapshots.php

View workflow job for this annotation

GitHub Actions / PHPCS 

file_get_contents() is discouraged. Use wp_remote_get() for remote URLs instead.
if ( false === $contents ) {
return array( 'success' => false, 'error' => 'Unable to read file: ' . $path );
}
Expand Down Expand Up @@ -155,7 +155,7 @@
'target' => $name,
'existed' => $existed,
),
$existed ? serialize( $value ) : ''

Check warning on line 158 in digitizer-site-worker/includes/class-aura-worker-snapshots.php

View workflow job for this annotation

GitHub Actions / PHPCS 

serialize() found. Serialized data has known vulnerability problems with Object Injection. JSON is generally a better approach for serializing data. See https://www.owasp.org/index.php/PHP_Object_Injection
);

if ( false === $record ) {
Expand Down Expand Up @@ -254,7 +254,7 @@
'target' => $post_id,
'keys' => array_map( 'strval', array_keys( $captured ) ),
),
serialize( $captured )

Check warning on line 257 in digitizer-site-worker/includes/class-aura-worker-snapshots.php

View workflow job for this annotation

GitHub Actions / PHPCS 

serialize() found. Serialized data has known vulnerability problems with Object Injection. JSON is generally a better approach for serializing data. See https://www.owasp.org/index.php/PHP_Object_Injection
);

if ( false === $record ) {
Expand Down Expand Up @@ -344,7 +344,7 @@
'kind' => 'posts',
'targets' => array_map( 'intval', array_keys( $captured ) ),
),
serialize( $captured )

Check warning on line 347 in digitizer-site-worker/includes/class-aura-worker-snapshots.php

View workflow job for this annotation

GitHub Actions / PHPCS 

serialize() found. Serialized data has known vulnerability problems with Object Injection. JSON is generally a better approach for serializing data. See https://www.owasp.org/index.php/PHP_Object_Injection
);
if ( false === $record ) {
return array( 'success' => false, 'error' => 'Failed to persist snapshot (disk full or unwritable).' );
Expand Down Expand Up @@ -391,10 +391,83 @@
if ( ! file_exists( $meta_path ) ) {
return null;
}
$data = json_decode( file_get_contents( $meta_path ), true );

Check warning on line 394 in digitizer-site-worker/includes/class-aura-worker-snapshots.php

View workflow job for this annotation

GitHub Actions / PHPCS 

file_get_contents() is discouraged. Use wp_remote_get() for remote URLs instead.
return is_array( $data ) ? $data : null;
}

/**
* Unserialize a snapshot payload without ever instantiating a class.
*
* The payload files are written by this plugin, but they sit on disk and are
* the one untrusted-if-tampered input on the restore path — a plain
* unserialize() would let a crafted payload build arbitrary objects and fire
* __wakeup()/__destruct() gadget chains. `allowed_classes => false` turns any
* serialized object into a __PHP_Incomplete_Class instead, closing that
* surface. Everything this engine actually captures — option values, the
* meta/post arrays it builds, Elementor's JSON-string payloads — is scalars
* and arrays, so nothing legitimate is lost. A payload that *did* contain an
* object was either tampered or a pathological object-valued option; the
* callers detect the resulting incomplete class (an object is not an array,
* and is_stripped_object() catches the top-level option case) and fail closed
* rather than write it back.
*
* @param string $raw Serialized payload bytes.
* @return mixed Unserialized value, or false on malformed input.
*/
private function unserialize_payload( $raw ) {
return unserialize( (string) $raw, array( 'allowed_classes' => false ) );

Check warning on line 418 in digitizer-site-worker/includes/class-aura-worker-snapshots.php

View workflow job for this annotation

GitHub Actions / PHPCS 

unserialize() found. Serialized data has known vulnerability problems with Object Injection. JSON is generally a better approach for serializing data. See https://www.owasp.org/index.php/PHP_Object_Injection
}

/**
* Maximum array nesting the stripped-object walk will descend before it
* treats the payload as unsafe. Real snapshot payloads (option values, the
* shallow meta/post maps, Elementor's JSON-string blobs) are nowhere near
* this deep, so a legitimate payload never reaches it; the cap exists only
* to bound a tampered one.
*/
private const MAX_WALK_DEPTH = 64;

/**
* True when a value is — or contains, at any depth — an object stripped by
* allowed_classes => false.
*
* The check must recurse: allowed_classes => false strips every serialized
* object to __PHP_Incomplete_Class, but a tampered payload can nest one
* inside an otherwise-valid array (e.g. serialize( array( 'x' => $gadget ) )).
* A top-level-only check passes that array straight through and update_option
* / update_post_meta persists the incomplete class as a "successful" restore.
* Walking the whole structure is what lets every kind fail closed on it.
*
* The walk is depth-bounded: unserialize() faithfully rebuilds a serialized
* reference cycle (e.g. `a:1:{i:0;R:1;}`) into a genuinely self-referential
* array, and an unbounded recursion over one would exhaust the stack and fatal
* the restore request rather than fail closed. Hitting the cap is itself
* treated as "unsafe" (return true) so a pathological payload is rejected, not
* restored.
*
* @param mixed $value Unserialized value.
* @param int $depth Current recursion depth (internal).
* @return bool
*/
private function contains_stripped_object( $value, $depth = 0 ) {
if ( $value instanceof \__PHP_Incomplete_Class ) {
return true;
}
if ( is_array( $value ) ) {
if ( $depth >= self::MAX_WALK_DEPTH ) {
// Too deep to be a real payload — a reference cycle or a crafted
// deeply-nested array. Refuse it rather than recurse into a fatal.
return true;
}
foreach ( $value as $item ) {
if ( $this->contains_stripped_object( $item, $depth + 1 ) ) {
return true;
}
}
}
return false;
}

/**
* Restore state from a snapshot.
*
Expand All @@ -413,7 +486,7 @@
if ( ! $payload_path || ! file_exists( $payload_path ) ) {
return array( 'success' => false, 'error' => 'Snapshot payload missing.' );
}
$ok = file_put_contents( $record['target'], file_get_contents( $payload_path ) );

Check warning on line 489 in digitizer-site-worker/includes/class-aura-worker-snapshots.php

View workflow job for this annotation

GitHub Actions / PHPCS 

File operations should use WP_Filesystem methods instead of direct PHP filesystem calls. Found: file_put_contents().
return ( false === $ok )
? array( 'success' => false, 'error' => 'Failed to write file: ' . $record['target'] )
: array( 'success' => true );
Expand All @@ -425,7 +498,16 @@
}
$payload_path = $record['payload_path'] ?? '';
$raw = ( $payload_path && file_exists( $payload_path ) ) ? file_get_contents( $payload_path ) : '';
update_option( $record['target'], unserialize( $raw ) );
$value = $this->unserialize_payload( $raw );
// Fail closed rather than write back an incomplete class: the payload
// held a serialized object (tampered, or a rare object-valued option),
// which allowed_classes => false intentionally refused to rebuild.
// Recurses so a nested object inside an array is caught too, not just
// a top-level one.
if ( $this->contains_stripped_object( $value ) ) {
return array( 'success' => false, 'error' => 'Snapshot payload contains an object and cannot be safely restored.' );
}
update_option( $record['target'], $value );

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Recursively reject stripped objects before persisting

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 👍 / 👎.

return array( 'success' => true );

case 'post':
Expand All @@ -452,8 +534,11 @@
if ( ! $payload_path || ! file_exists( $payload_path ) ) {
return array( 'success' => false, 'error' => 'Snapshot payload missing.' );
}
$captured = unserialize( file_get_contents( $payload_path ) );
if ( ! is_array( $captured ) ) {
$captured = $this->unserialize_payload( file_get_contents( $payload_path ) );
// A tampered payload that serialized an object is stripped to an
// incomplete class. Reject both a top-level one (not an array) and one
// nested inside a captured meta value, before any of it is written.
if ( ! is_array( $captured ) || $this->contains_stripped_object( $captured ) ) {
return array( 'success' => false, 'error' => 'Snapshot payload corrupt.' );
}
$post_id = (int) $record['target'];
Expand All @@ -472,8 +557,11 @@
if ( ! $payload_path || ! file_exists( $payload_path ) ) {
return array( 'success' => false, 'error' => 'Snapshot payload missing.' );
}
$captured = unserialize( file_get_contents( $payload_path ) );
if ( ! is_array( $captured ) ) {
$captured = $this->unserialize_payload( file_get_contents( $payload_path ) );
// A tampered payload that serialized an object is stripped to an
// incomplete class. Reject both a top-level one (not an array) and one
// nested inside a captured post/meta value, before any of it is written.
if ( ! is_array( $captured ) || $this->contains_stripped_object( $captured ) ) {
return array( 'success' => false, 'error' => 'Snapshot payload corrupt.' );
}
foreach ( $captured as $pid => $info ) {
Expand Down
163 changes: 163 additions & 0 deletions tests/unit/SnapshotsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,4 +404,167 @@ public function test_posts_snapshot_rejects_empty_ids(): void {
$snaps = new Aura_Worker_Snapshots();
$this->assertFalse( $snaps->snapshot_posts( array(), '_x' )['success'] );
}

// --- object-injection hardening on the restore path ---------------------
//
// Payload files are written by the plugin, but they are the one on-disk,
// tamperable input restore() consumes. A plain unserialize() there would
// build arbitrary objects and fire __wakeup()/__destruct() gadget chains, so
// every payload is unserialized with allowed_classes => false. These tests
// pin the security property (no class is ever instantiated) and prove the
// scalar/array payloads the engine actually uses still round-trip.

/** Overwrite a snapshot's payload file with attacker-chosen bytes. */
private function tamperPayload( array $snapshot, string $bytes ): void {
$this->assertNotEmpty( $snapshot['payload_path'] ?? '' );
file_put_contents( $snapshot['payload_path'], $bytes );
}

public function test_object_payload_never_instantiates_a_class_on_restore(): void {
Aura_Snapshot_Gadget::$woke = false;

update_option( 'gadget_opt', 'benign' );
$snaps = new Aura_Worker_Snapshots();
$snap = $snaps->snapshot_option( 'gadget_opt' );

// Attacker replaces the payload with a serialized gadget object.
$this->tamperPayload( $snap['snapshot'], serialize( new Aura_Snapshot_Gadget() ) );

$res = $snaps->restore( $snap['snapshot']['id'] );

$this->assertFalse( Aura_Snapshot_Gadget::$woke, 'allowed_classes=false must prevent __wakeup() from ever firing.' );
$this->assertFalse( $res['success'], 'An object payload is refused, not written back.' );
$this->assertStringContainsString( 'object', $res['error'] );
// The live option is untouched — no incomplete class leaked into storage.
$this->assertSame( 'benign', get_option( 'gadget_opt' ) );
}

public function test_meta_object_payload_is_rejected_as_corrupt(): void {
Aura_Snapshot_Gadget::$woke = false;

$this->seedPost( 88 );
update_post_meta( 88, '_elementor_data', 'orig' );
$snaps = new Aura_Worker_Snapshots();
$snap = $snaps->snapshot_meta( 88, '_elementor_data' );

$this->tamperPayload( $snap['snapshot'], serialize( new Aura_Snapshot_Gadget() ) );

$res = $snaps->restore( $snap['snapshot']['id'] );

$this->assertFalse( Aura_Snapshot_Gadget::$woke );
$this->assertFalse( $res['success'] );
$this->assertStringContainsString( 'corrupt', $res['error'] );
}

public function test_posts_object_payload_is_rejected_as_corrupt(): void {
Aura_Snapshot_Gadget::$woke = false;

$this->seedClassPost( 601, '{"v":1}' );
$snaps = new Aura_Worker_Snapshots();
$snap = $snaps->snapshot_posts( array( 601 ), '_elementor_global_class_data' );

$this->tamperPayload( $snap['snapshot'], serialize( new Aura_Snapshot_Gadget() ) );

$res = $snaps->restore( $snap['snapshot']['id'] );

$this->assertFalse( Aura_Snapshot_Gadget::$woke );
$this->assertFalse( $res['success'] );
$this->assertStringContainsString( 'corrupt', $res['error'] );
}

public function test_option_nested_object_payload_is_rejected(): void {
// A gadget hidden INSIDE an array: allowed_classes=false strips it to an
// incomplete class, the top level stays an array, so a top-level-only guard
// would store it as a successful restore. The recursive check must catch it.
Aura_Snapshot_Gadget::$woke = false;

update_option( 'nested_opt', 'benign' );
$snaps = new Aura_Worker_Snapshots();
$snap = $snaps->snapshot_option( 'nested_opt' );

$this->tamperPayload( $snap['snapshot'], serialize( array( 'x' => new Aura_Snapshot_Gadget() ) ) );

$res = $snaps->restore( $snap['snapshot']['id'] );

$this->assertFalse( Aura_Snapshot_Gadget::$woke );
$this->assertFalse( $res['success'], 'A nested object payload must not restore.' );
$this->assertStringContainsString( 'object', $res['error'] );
// Nothing corrupt leaked into storage.
$this->assertSame( 'benign', get_option( 'nested_opt' ) );
}

public function test_meta_nested_object_payload_is_rejected(): void {
// snapshot_meta serializes array( key => array( 'existed'=>.., 'value'=>.. ) ),
// so a gadget in a leaf value is the realistic nesting for this kind.
Aura_Snapshot_Gadget::$woke = false;

$this->seedPost( 91 );
update_post_meta( 91, '_elementor_data', 'orig' );
$snaps = new Aura_Worker_Snapshots();
$snap = $snaps->snapshot_meta( 91, '_elementor_data' );

$payload = array( '_elementor_data' => array( 'existed' => true, 'value' => new Aura_Snapshot_Gadget() ) );
$this->tamperPayload( $snap['snapshot'], serialize( $payload ) );

$res = $snaps->restore( $snap['snapshot']['id'] );

$this->assertFalse( Aura_Snapshot_Gadget::$woke );
$this->assertFalse( $res['success'] );
$this->assertStringContainsString( 'corrupt', $res['error'] );
// The live meta was not overwritten with a stripped class.
$this->assertSame( 'orig', get_post_meta( 91, '_elementor_data', true ) );
}

public function test_cyclic_array_payload_fails_closed_without_fataling(): void {
// unserialize() rebuilds a serialized reference cycle into a genuinely
// self-referential array; the stripped-object walk must not recurse into
// it forever (stack exhaustion / DoS) but reject it fast.
update_option( 'cyclic_opt', 'benign' );
$snaps = new Aura_Worker_Snapshots();
$snap = $snaps->snapshot_option( 'cyclic_opt' );

// a:2:{s:1:"k";i:1;s:4:"self";R:1;} — element 'self' points back at the array.
$cyclic = 'a:2:{s:1:"k";i:1;s:4:"self";R:1;}';
$this->tamperPayload( $snap['snapshot'], $cyclic );

$start = microtime( true );
$res = $snaps->restore( $snap['snapshot']['id'] );
$elapsed = microtime( true ) - $start;

$this->assertFalse( $res['success'], 'A cyclic payload must be refused, not restored.' );
$this->assertStringContainsString( 'object', $res['error'] );
$this->assertLessThan( 1.0, $elapsed, 'The walk must terminate, not spin on the cycle.' );
$this->assertSame( 'benign', get_option( 'cyclic_opt' ) );
}

public function test_scalar_and_array_option_payloads_still_round_trip(): void {
// The hardening must not regress the real payloads: scalars and nested
// arrays are exactly what options and Elementor meta hold.
$snaps = new Aura_Worker_Snapshots();

update_option( 'scalar_opt', 'a string' );
$s1 = $snaps->snapshot_option( 'scalar_opt' );
update_option( 'scalar_opt', 'changed' );
$this->assertTrue( $snaps->restore( $s1['snapshot']['id'] )['success'] );
$this->assertSame( 'a string', get_option( 'scalar_opt' ) );

update_option( 'array_opt', array( 'n' => 1, 'deep' => array( 'x', 'y' ) ) );
$s2 = $snaps->snapshot_option( 'array_opt' );
update_option( 'array_opt', array( 'n' => 2 ) );
$this->assertTrue( $snaps->restore( $s2['snapshot']['id'] )['success'] );
$this->assertSame( array( 'n' => 1, 'deep' => array( 'x', 'y' ) ), get_option( 'array_opt' ) );
}
}

/**
* A stand-in object-injection gadget: if a plain unserialize() ever rebuilt it,
* __wakeup() would flip the static flag. allowed_classes => false must keep that
* flag false. Defined at file scope so serialize()/unserialize() can name it.
*/
final class Aura_Snapshot_Gadget {
public static bool $woke = false;

public function __wakeup(): void {
self::$woke = true;
}
}
Loading