diff --git a/changelog.txt b/changelog.txt index d60ff345..58394ba2 100644 --- a/changelog.txt +++ b/changelog.txt @@ -3,6 +3,8 @@ = 4.1.0 - xxxx-xx-xx = * Use `wp_admin_notice()` to render admin notices, instead of custom HTML. * Fix an oversight in the lock implementation (used to rate-limit async request runners) that could result in a database error in unusual cases. +* Add protections to guard against the risk of object-injection/deserialization attacks when retrieving stored schedule data. +* Dev - Add filters (`action_scheduler_allowed_nested_schedule_classes`, `action_scheduler_enforce_schedule_allowed_classes`) and an action (`action_scheduler_unexpected_schedule_class`) to tune and observe schedule deserialization. * Dev - Updates the `wpPostStore` to capture any database errors that might occur while claiming actions. = 4.0.0 - 2026-06-16 = diff --git a/classes/ActionScheduler_ScheduleDeserializer.php b/classes/ActionScheduler_ScheduleDeserializer.php new file mode 100644 index 00000000..ffd142b0 --- /dev/null +++ b/classes/ActionScheduler_ScheduleDeserializer.php @@ -0,0 +1,412 @@ + + */ + private $seen = array(); + + /** + * Unexpected/disallowed class names discovered nested in the blob. + * + * @var string[] + */ + private $potential_offenders; + + /** + * Safely deserialize a stored schedule blob without instantiating unexpected classes. + * + * @param mixed $blob The raw serialized schedule blob as stored in the database. + * + * @return ActionScheduler_Schedule|false + */ + public static function unserialize( $blob ) { + /** + * Filters the list of classes that are permitted as nested properties of a schedule object. + * + * The result of this filter is not memoized: this is deliberate, so that it can be changed + * if the context (including the current blog, in a multisite network) alters during the + * same request. + * + * @since 4.1.0 + * + * @param string[] $default List of allowed nested class names. + */ + $allowed_nested = (array) apply_filters( 'action_scheduler_allowed_nested_schedule_classes', self::DEFAULT_NESTED_CLASSES ); + $deserializer = new self( self::KNOWN_SCHEDULE_CLASSES, $allowed_nested ); + return $deserializer( $blob ); + } + + /** + * Build our ActionScheduler_ScheduleDeserializer instance. + * + * @param array $allowed_scheduler_classes Scheduler classes we accept. + * @param array $allowed_nested_classes Nested classes we accept. + */ + public function __construct( array $allowed_scheduler_classes, array $allowed_nested_classes ) { + $this->allowed_scheduler_classes = $allowed_scheduler_classes; + $this->allowed_nested_classes = $allowed_nested_classes; + } + + /** + * Deserialize a blob of serialized data. + * + * @param mixed $serialized_blob Data to unserialize. Expected to be a string. + * + * @return ActionScheduler_Schedule|false + */ + public function __invoke( $serialized_blob ) { + if ( ! is_string( $serialized_blob ) || '' === $serialized_blob ) { + return false; + } + + $this->seen = array(); + $this->potential_offenders = array(); + $this->source_data = $serialized_blob; + + try { + return $this->deserialize(); + } catch ( Throwable $e ) { + return false; + } + } + + /** + * Perform the two-pass deserialization for this instance's already-validated blob. + * + * @return ActionScheduler_Schedule|false + */ + private function deserialize() { + // Initial attempt to unserialize, specifying all trusted classes: for default actions using + // supplied schedule classes, this should succeed without any problems. Otherwise, the result + // may contain one or more __PHP_Incomplete_Class references that we will need to examine. + $all_trusted_classes = array_merge( $this->allowed_scheduler_classes, $this->allowed_nested_classes ); + $candidate = @unserialize( $this->source_data, array( 'allowed_classes' => $all_trusted_classes ) ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_unserialize, WordPress.PHP.NoSilencedErrors.Discouraged + + // A schedule is always an object graph. + if ( ! is_object( $candidate ) ) { + return false; + } + + $this->outer_class = $this->class_name_of( $candidate ); + + // Verify that we have a valid schedule representation, scanning for disallowed nested classes. + if ( ! $this->is_schedule_implementation( $this->outer_class ) || ! $this->scan_object_graph( $candidate, true, 0 ) ) { + return $this->reject( $this->outer_class, false ); + } + + // If the candidate is an acceptable schedule class, and doesn't contain any potential offenders, it is safe. + if ( ! $candidate instanceof __PHP_Incomplete_Class && empty( $this->potential_offenders ) ) { + return $candidate; + } + + // Otherwise the top-level object is itself a valid schedule, but references one or more disallowed classes. + foreach ( $this->potential_offenders as $offender ) { + if ( ! $this->is_allowed_nested_class( $offender, $this->allowed_nested_classes ) ) { + return $this->reject( $offender, true ); + } + } + + // If we reach this point, any potential offenders must in fact be safe. + $allowed = array_values( + array_unique( + array_merge( $all_trusted_classes, array( $this->outer_class ), $this->potential_offenders ) + ) + ); + + // Re-run unserialize. + return @unserialize( $this->source_data, array( 'allowed_classes' => $allowed ) ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_unserialize, WordPress.PHP.NoSilencedErrors.Discouraged + } + + /** + * Walk the parsed graph once, recording unexpected (placeholder) classes and guarding traversal. + * + * Objects whose class was outside the safe set are __PHP_Incomplete_Class placeholders; their + * original names are collected into $this->potential_offenders (the root is excluded — it is tracked + * separately as $this->outer_class). Real objects are trusted and only recursed into so a placeholder + * nested inside one is still found. A tampered blob can decode (via `r:`/`R:` references) into a + * self-referential or extremely deep graph, so visited objects are tracked to short-circuit cycles + * and shared references, and recursion depth is capped. + * + * @param mixed $value The value to walk (object, array, or scalar). + * @param bool $is_root Whether $value is the outermost object (excluded from $this->potential_offenders). + * @param int $depth Current recursion depth. + * + * @return bool True if the value was fully walked; false if it was too deep to vet safely. + */ + private function scan_object_graph( $value, bool $is_root, int $depth ): bool { + if ( $depth > self::MAX_GRAPH_DEPTH ) { + return false; + } + + if ( is_object( $value ) ) { + $object_id = spl_object_id( $value ); + if ( isset( $this->seen[ $object_id ] ) ) { + return true; // Already walked (cycle or shared reference); nothing new to collect. + } + $this->seen[ $object_id ] = true; + + $is_placeholder = $value instanceof __PHP_Incomplete_Class; + if ( $is_placeholder && ! $is_root ) { + $this->potential_offenders[] = $this->class_name_of( $value ); + } + + foreach ( (array) $value as $key => $child ) { + if ( $is_placeholder && '__PHP_Incomplete_Class_Name' === $key ) { + continue; + } + if ( ! $this->scan_object_graph( $child, false, $depth + 1 ) ) { + return false; + } + } + + return true; + } + + if ( is_array( $value ) ) { + foreach ( $value as $child ) { + if ( ! $this->scan_object_graph( $child, false, $depth + 1 ) ) { + return false; + } + } + } + + return true; + } + + /** + * Handle a blob that references a class outside the vetted set. + * + * Always surfaces the event for observability. Under enforcement (the default) the blob is + * rejected by returning false, which callers already handle like a corrupt schedule. + * + * Shadow mode ("report only") relaxes this, but only for a $shadow_eligible rejection — one where + * the outermost object is a valid schedule and merely a *nested* class was unexpected. That is the + * sole case where a mis-tuned allow-list could disrupt an otherwise-legitimate action, so it is the + * only case shadow mode lets through. Structural rejections ($shadow_eligible false) — a top-level + * non-schedule (the issue #1318 PoC) or an unwalkable object graph — have no legitimate form and are + * always rejected, even in shadow mode, so report-only can never be downgraded into reviving the + * vulnerability. + * + * @param string $offending_class The class that triggered the rejection. + * @param bool $shadow_eligible Whether shadow mode may let this particular rejection through. + * @return ActionScheduler_Schedule|false False when rejected. A shadow-mode passthrough is only + * reached for a nested rejection, i.e. after the top-level + * schedule gate passed, so any object returned is a schedule. + */ + protected function reject( $offending_class, $shadow_eligible ) { + // A rejection stands unless we are in shadow mode AND this rejection is eligible for report-only + // passthrough. Resolve it once so the reported outcome and the branch taken cannot disagree, and + // the action_scheduler_enforce_schedule_allowed_classes filter runs a single time. + $rejected = self::is_enforced() || ! $shadow_eligible; + + /** + * Fires when a stored schedule blob references a class Action Scheduler did not expect. + * + * @since 4.1.0 + * + * @param string $offending_class The class that triggered the rejection. + * @param string $outer_class The outermost class in the blob. + * @param string[] $unexpected_classes Every class the blob names that is outside the safe set. + * @param bool $rejected Whether the blob was rejected (true) or allowed through in + * shadow mode (false). Always true for a structural rejection + * (top-level non-schedule or unwalkable graph), which shadow + * mode cannot relax. + */ + do_action( 'action_scheduler_unexpected_schedule_class', $offending_class, $this->outer_class, $this->potential_offenders, $rejected ); + + if ( $rejected ) { + return false; + } + + // Shadow mode, nested-only rejection: behave as Action Scheduler did before this change. This is + // reached only when the top-level object is a valid schedule, so the unrestricted parse can no + // longer instantiate a bare top-level gadget. + return @unserialize( $this->source_data ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_unserialize, WordPress.PHP.NoSilencedErrors.Discouraged + } + + /** + * Decide whether the outermost class in a schedule blob may be instantiated. + * + * Allowed if the class is loaded and implements ActionScheduler_Schedule. A class that is not + * loaded cannot be validated (and, prior to this change, already produced a broken action), so + * rejecting it here does not regress any case that previously worked. + * + * @param string $class_name Class name discovered in the blob. + * @return bool + */ + private function is_schedule_implementation( string $class_name ): bool { + if ( '' === $class_name || ! class_exists( $class_name ) ) { + return false; + } + + $implements = class_implements( $class_name ); + return is_array( $implements ) && isset( $implements['ActionScheduler_Schedule'] ); + } + + /** + * Decide whether a nested class (a property of the schedule) may be instantiated. + * + * The only classes Action Scheduler itself ever nests inside a schedule are the bundled + * CronExpression family. Composite schedules that nest another schedule are also fine. The + * default list is filterable so extenders can vet their own supporting classes. + * + * @param string $class_name Class name discovered nested in the blob. + * @param string[] $allowed_nested The resolved nested allow-list to check against. + * + * @return bool + */ + private function is_allowed_nested_class( string $class_name, array $allowed_nested ): bool { + if ( '' === $class_name ) { + return false; + } + + if ( in_array( $class_name, $allowed_nested, true ) ) { + return true; + } + + // A schedule nested inside another schedule (composite schedules) is safe by the same rule + // as the top-level check. + if ( class_exists( $class_name ) ) { + $implements = class_implements( $class_name ); + if ( is_array( $implements ) && isset( $implements['ActionScheduler_Schedule'] ) ) { + return true; + } + } + + return false; + } + + /** + * Get the class name of an object parsed with a restricted allowed_classes set. + * + * Objects whose class was disallowed become __PHP_Incomplete_Class, which hides the original + * name behind get_class(); it lives in the __PHP_Incomplete_Class_Name pseudo-property instead. + * stdClass is never converted by PHP, so its real name is returned directly. + * + * @param object $maybe_incomplete Object parsed with a restricted allowed_classes set. + * @return string The original class name, or '' if it cannot be determined. + */ + private function class_name_of( object $maybe_incomplete ): string { + if ( $maybe_incomplete instanceof __PHP_Incomplete_Class ) { + $vars = (array) $maybe_incomplete; + return isset( $vars['__PHP_Incomplete_Class_Name'] ) ? (string) $vars['__PHP_Incomplete_Class_Name'] : ''; + } + + return get_class( $maybe_incomplete ); + } + + /** + * Whether an unexpected class causes the blob to be rejected (true) or merely reported while the + * legacy, unrestricted deserialization proceeds (false, "shadow mode"). + * + * Enforcement is on by default. Site owners can opt into shadow mode for a release to gather + * data before enforcing, without changing any stored data. + * + * @return bool + */ + public static function is_enforced(): bool { + /** + * Filters whether Action Scheduler rejects schedule blobs referencing unexpected classes. + * + * Return false to run in "shadow mode": unexpected classes are reported via the + * `action_scheduler_unexpected_schedule_class` action but the blob is still deserialized + * exactly as it was before this hardening. Useful to confirm the allow-list is complete for + * your site before switching enforcement on. + * + * @since 4.1.0 + * + * @param bool $enforced Whether to reject blobs with unexpected classes. Default true. + */ + return (bool) apply_filters( 'action_scheduler_enforce_schedule_allowed_classes', true ); + } +} diff --git a/classes/data-stores/ActionScheduler_DBStore.php b/classes/data-stores/ActionScheduler_DBStore.php index 96da02af..41b9e50d 100644 --- a/classes/data-stores/ActionScheduler_DBStore.php +++ b/classes/data-stores/ActionScheduler_DBStore.php @@ -401,7 +401,7 @@ protected function make_action_from_db_record( $data ) { $this->validate_args( $args, $data->action_id ); } - $schedule = @unserialize( $data->schedule ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_unserialize WordPress.PHP.NoSilencingOperator + $schedule = ActionScheduler_ScheduleDeserializer::unserialize( $data->schedule ); if ( false === $schedule && $data->status === self::STATUS_CANCELED ) { // The handled corrupted action should appear in UI. $schedule = new ActionScheduler_NullSchedule(); diff --git a/classes/data-stores/ActionScheduler_wpPostStore.php b/classes/data-stores/ActionScheduler_wpPostStore.php index a72df963..8703713c 100644 --- a/classes/data-stores/ActionScheduler_wpPostStore.php +++ b/classes/data-stores/ActionScheduler_wpPostStore.php @@ -238,7 +238,7 @@ protected function make_action_from_post( $post ) { $args = json_decode( $post->post_content, true ); $this->validate_args( $args, $post->ID ); - $schedule = get_post_meta( $post->ID, self::SCHEDULE_META_KEY, true ); + $schedule = $this->get_schedule_from_post_meta( $post->ID ); $this->validate_schedule( $schedule, $post->ID ); $group = wp_get_object_terms( $post->ID, self::GROUP_TAXONOMY, array( 'fields' => 'names' ) ); @@ -247,6 +247,45 @@ protected function make_action_from_post( $post ) { return ActionScheduler::factory()->get_stored_action( $this->get_action_status_by_post_status( $post->post_status ), $hook, $args, $schedule, $group ); } + /** + * Read a scheduled action's schedule from post meta without instantiating unexpected classes. + * + * Using get_post_meta() would run the stored blob through maybe_unserialize(), instantiating + * whatever classes it names before we can vet them. Instead we read the raw serialized value and + * hand it to ActionScheduler_ScheduleDeserializer, which only instantiates a valid schedule (and its + * vetted supporting classes). @see https://github.com/woocommerce/action-scheduler/issues/1318 + * + * @param int $post_id Post ID of the scheduled action. + * @return mixed The schedule object, false if unusable, or the raw meta value when it is not a + * serialized object (left for validate_schedule() to reject, as before). + */ + protected function get_schedule_from_post_meta( $post_id ) { + global $wpdb; + + // Direct, uncached query is intentional: we need the raw serialized value before the meta API + // runs it through maybe_unserialize(), so we bypass get_post_meta() and its cache here. + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching + $raw = $wpdb->get_var( + $wpdb->prepare( + "SELECT meta_value FROM {$wpdb->postmeta} WHERE post_id = %d AND meta_key = %s ORDER BY meta_id ASC LIMIT 1", + $post_id, + self::SCHEDULE_META_KEY + ) + ); + + if ( null === $raw ) { + return false; + } + + // Schedule objects are stored serialized by the meta API. A non-serialized value cannot be a + // schedule object, so there is nothing to protect against — return it as-is for validation. + if ( ! is_serialized( $raw ) ) { + return $raw; + } + + return ActionScheduler_ScheduleDeserializer::unserialize( $raw ); + } + /** * Get action status by post status. * diff --git a/tests/bootstrap.php b/tests/bootstrap.php index ba6c31b7..df33ffac 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -32,6 +32,10 @@ } include_once 'phpunit/helpers/ActionScheduler_Callbacks.php'; +include_once 'phpunit/helpers/ActionScheduler_Test_Evil_Gadget.php'; +include_once 'phpunit/helpers/ActionScheduler_Test_Custom_Schedule.php'; +include_once 'phpunit/helpers/ActionScheduler_Test_Schedule_Helper.php'; +include_once 'phpunit/helpers/ActionScheduler_Test_Custom_Schedule_With_Property.php'; include_once 'phpunit/ActionScheduler_Mocker.php'; include_once 'phpunit/ActionScheduler_Mock_Async_Request_QueueRunner.php'; include_once 'phpunit/jobstore/AbstractStoreTest.php'; diff --git a/tests/phpunit.xml.dist b/tests/phpunit.xml.dist index 65ee747b..721cbbf4 100644 --- a/tests/phpunit.xml.dist +++ b/tests/phpunit.xml.dist @@ -17,6 +17,7 @@ ./phpunit/jobstore/ActionScheduler_DBStoreMigrator_Test.php ./phpunit/jobstore/ActionScheduler_DBStore_Test.php + ./phpunit/jobstore/ActionScheduler_ScheduleUnserialize_Test.php ./phpunit/jobstore/ActionScheduler_HybridStore_Test.php ./phpunit/logging/ActionScheduler_DBLogger_Test.php diff --git a/tests/phpunit/helpers/ActionScheduler_Test_Custom_Schedule.php b/tests/phpunit/helpers/ActionScheduler_Test_Custom_Schedule.php new file mode 100644 index 00000000..3adb6ea8 --- /dev/null +++ b/tests/phpunit/helpers/ActionScheduler_Test_Custom_Schedule.php @@ -0,0 +1,44 @@ +timestamp = $timestamp; + } + + /** + * Get the next run date. + * + * @param null|DateTime $after Timestamp. + * @return DateTime|null + */ + public function next( ?DateTime $after = null ) { + return as_get_datetime_object( $this->timestamp ); + } + + /** + * Not recurring. + * + * @return bool + */ + public function is_recurring() { + return false; + } +} diff --git a/tests/phpunit/helpers/ActionScheduler_Test_Custom_Schedule_With_Property.php b/tests/phpunit/helpers/ActionScheduler_Test_Custom_Schedule_With_Property.php new file mode 100644 index 00000000..0fbc4702 --- /dev/null +++ b/tests/phpunit/helpers/ActionScheduler_Test_Custom_Schedule_With_Property.php @@ -0,0 +1,72 @@ +at = new DateTime( '@' . $timestamp ); + $this->created = new DateTimeImmutable( '@' . $timestamp ); + $this->zone = new DateTimeZone( 'UTC' ); + $this->every = new DateInterval( 'P1D' ); + $this->helper = $helper; + } + + /** + * Get the next run date. + * + * @param null|DateTime $after Timestamp. + * @return DateTime|null + */ + public function next( ?DateTime $after = null ) { + return as_get_datetime_object( $this->at->getTimestamp() ); + } + + /** + * Recurring. + * + * @return bool + */ + public function is_recurring() { + return true; + } +} diff --git a/tests/phpunit/helpers/ActionScheduler_Test_Evil_Gadget.php b/tests/phpunit/helpers/ActionScheduler_Test_Evil_Gadget.php new file mode 100644 index 00000000..f9da0e4f --- /dev/null +++ b/tests/phpunit/helpers/ActionScheduler_Test_Evil_Gadget.php @@ -0,0 +1,26 @@ +value = $value; + } +} diff --git a/tests/phpunit/jobstore/ActionScheduler_ScheduleUnserialize_Test.php b/tests/phpunit/jobstore/ActionScheduler_ScheduleUnserialize_Test.php new file mode 100644 index 00000000..ecdbaeb1 --- /dev/null +++ b/tests/phpunit/jobstore/ActionScheduler_ScheduleUnserialize_Test.php @@ -0,0 +1,589 @@ +query( "DELETE FROM {$wpdb->actionscheduler_actions}" ); + ActionScheduler_Test_Evil_Gadget::$fired = false; + parent::setUp(); + } + + /** + * Persist a real action, then overwrite its stored schedule blob with an arbitrary string, + * simulating an attacker (or corruption) tampering with the serialized column. + * + * @param string $schedule_blob Raw value to store in the schedule column. + * @return int Action ID. + */ + private function store_action_with_raw_schedule( $schedule_blob ) { + global $wpdb; + + $store = new ActionScheduler_DBStore(); + $action = new ActionScheduler_Action( + ActionScheduler_Callbacks::HOOK_WITH_CALLBACK, + array(), + new ActionScheduler_SimpleSchedule( as_get_datetime_object() ) + ); + $action_id = $store->save_action( $action ); + + $wpdb->update( + $wpdb->actionscheduler_actions, + array( 'schedule' => $schedule_blob ), + array( 'action_id' => $action_id ), + array( '%s' ), + array( '%d' ) + ); + + return $action_id; + } + + /** + * THE VULNERABILITY. A tampered schedule blob referencing an arbitrary class must never + * cause that class to be instantiated during deserialization. + * + * On trunk this test is RED: unserialize() instantiates the gadget and runs its __wakeup(). + */ + public function test_arbitrary_class_is_not_instantiated_from_schedule_blob() { + $malicious = serialize( new ActionScheduler_Test_Evil_Gadget() ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize + $action_id = $this->store_action_with_raw_schedule( $malicious ); + + $store = new ActionScheduler_DBStore(); + + // Fetching may legitimately throw/return a null action for an unusable schedule; that is + // fine. What must never happen is the gadget's __wakeup() side effect firing. + try { + $store->fetch_action( $action_id ); + } catch ( Exception $e ) { + unset( $e ); // Rejecting the action is an acceptable outcome; we only assert on the side effect. + } + + $this->assertFalse( + ActionScheduler_Test_Evil_Gadget::$fired, + 'Deserializing a schedule blob instantiated an arbitrary class (object injection).' + ); + } + + /** + * A gadget nested INSIDE an otherwise-valid schedule wrapper must also be blocked. This guards + * against smuggling a gadget in via the recurrence/property of a legitimate schedule class. + */ + public function test_arbitrary_class_nested_in_valid_schedule_is_not_instantiated() { + $nul = chr( 0 ); + $prop_ts = $nul . '*' . $nul . 'scheduled_timestamp'; // Protected property marker. + $prop_rec = $nul . '*' . $nul . 'recurrence'; // Protected property marker. + $gadget = 'ActionScheduler_Test_Evil_Gadget'; + $container = 'ActionScheduler_IntervalSchedule'; + $blob = 'O:' . strlen( $container ) . ':"' . $container . '":2:{' + . 's:' . strlen( $prop_ts ) . ':"' . $prop_ts . '";i:' . ( time() + HOUR_IN_SECONDS ) . ';' + . 's:' . strlen( $prop_rec ) . ':"' . $prop_rec . '";' + . 'O:' . strlen( $gadget ) . ':"' . $gadget . '":0:{}' + . '}'; + $action_id = $this->store_action_with_raw_schedule( $blob ); + + $store = new ActionScheduler_DBStore(); + try { + $store->fetch_action( $action_id ); + } catch ( Exception $e ) { + unset( $e ); // Rejecting the action is an acceptable outcome; we only assert on the side effect. + } + + $this->assertFalse( + ActionScheduler_Test_Evil_Gadget::$fired, + 'A gadget nested inside a valid schedule wrapper was instantiated.' + ); + } + + /** + * A self-referential blob must not send the class-name walk into infinite recursion. + * + * PHP's `r:` references let a tampered blob decode into a cyclic object graph. Before the cycle + * guard, walking it recursed until the stack overflowed (a DoS). The store must instead return. + */ + public function test_self_referential_blob_is_handled_without_infinite_recursion() { + $nul = chr( 0 ); + $prop_rec = $nul . '*' . $nul . 'recurrence'; // Protected property marker. + $container = 'ActionScheduler_IntervalSchedule'; + // The single property `recurrence` references value #1 — the object itself — forming a cycle. + $blob = 'O:' . strlen( $container ) . ':"' . $container . '":1:{' + . 's:' . strlen( $prop_rec ) . ':"' . $prop_rec . '";r:1;}'; + $action_id = $this->store_action_with_raw_schedule( $blob ); + + $store = new ActionScheduler_DBStore(); + $fetched = null; + try { + $fetched = $store->fetch_action( $action_id ); + } catch ( Exception $e ) { + unset( $e ); // Acceptable; the point is that we return rather than exhaust the stack. + } + + // Reaching this line at all proves the walk terminated. fetch_action always yields an object. + $this->assertNotNull( $fetched, 'A self-referential schedule blob did not resolve to an action.' ); + } + + /** + * The happy path must keep working: a simple schedule round-trips through the store unchanged. + */ + public function test_simple_schedule_round_trips() { + $time = as_get_datetime_object( '2026-03-04 05:06:07' ); + $store = new ActionScheduler_DBStore(); + $action = new ActionScheduler_Action( ActionScheduler_Callbacks::HOOK_WITH_CALLBACK, array(), new ActionScheduler_SimpleSchedule( $time ) ); + $action_id = $store->save_action( $action ); + + $fetched = $store->fetch_action( $action_id ); + $schedule = $fetched->get_schedule(); + + $this->assertInstanceOf( 'ActionScheduler_SimpleSchedule', $schedule ); + $this->assertEquals( $time->getTimestamp(), $schedule->get_date()->getTimestamp() ); + } + + /** + * Cron schedules must round-trip, including their recurrence. + */ + public function test_cron_schedule_round_trips() { + $time = as_get_datetime_object( '2026-01-01 00:00:00' ); + $store = new ActionScheduler_DBStore(); + $action = new ActionScheduler_Action( ActionScheduler_Callbacks::HOOK_WITH_CALLBACK, array(), new ActionScheduler_CronSchedule( $time, '0 0 * * *' ) ); + $action_id = $store->save_action( $action ); + + $fetched = $store->fetch_action( $action_id ); + $schedule = $fetched->get_schedule(); + + $this->assertInstanceOf( 'ActionScheduler_CronSchedule', $schedule ); + $this->assertEquals( '0 0 * * *', $schedule->get_recurrence() ); + } + + /** + * Existing rows serialized before this change still contain the full CronExpression object + * graph. Those must continue to deserialize into a working schedule. + */ + public function test_legacy_cron_blob_still_unserializes() { + $action_id = $this->store_action_with_raw_schedule( base64_decode( self::LEGACY_CRON_BLOB_B64 ) ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.obfuscation_base64_decode + + $store = new ActionScheduler_DBStore(); + $fetched = $store->fetch_action( $action_id ); + $schedule = $fetched->get_schedule(); + + $this->assertInstanceOf( 'ActionScheduler_CronSchedule', $schedule ); + $this->assertEquals( '0 0 * * *', $schedule->get_recurrence() ); + } + + /** + * A third party schedule class (implementing ActionScheduler_Schedule, defined outside AS) + * must keep working without the vendor registering anything. + */ + public function test_third_party_schedule_class_round_trips() { + $timestamp = time() + DAY_IN_SECONDS; + $blob = serialize( new ActionScheduler_Test_Custom_Schedule( $timestamp ) ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize + $action_id = $this->store_action_with_raw_schedule( $blob ); + + $store = new ActionScheduler_DBStore(); + $fetched = $store->fetch_action( $action_id ); + $schedule = $fetched->get_schedule(); + + $this->assertInstanceOf( 'ActionScheduler_Test_Custom_Schedule', $schedule ); + $this->assertEquals( $timestamp, $schedule->next()->getTimestamp() ); + } + + /** + * The same protection must cover the legacy post-based store, whose schedule lives in post meta. + * + * On trunk this is RED: get_post_meta() runs the tampered blob through maybe_unserialize(). + */ + public function test_wp_post_store_does_not_instantiate_arbitrary_class() { + $store = new ActionScheduler_wpPostStore(); + $action = new ActionScheduler_Action( + ActionScheduler_Callbacks::HOOK_WITH_CALLBACK, + array(), + new ActionScheduler_SimpleSchedule( as_get_datetime_object() ) + ); + $action_id = $store->save_action( $action ); + + // Tamper the stored schedule meta with a gadget blob, bypassing the meta API's serialization. + global $wpdb; + // phpcs:disable WordPress.DB.SlowDBQuery.slow_db_query_meta_value, WordPress.DB.SlowDBQuery.slow_db_query_meta_key, WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize + $wpdb->update( + $wpdb->postmeta, + array( 'meta_value' => serialize( new ActionScheduler_Test_Evil_Gadget() ) ), + array( + 'post_id' => $action_id, + 'meta_key' => ActionScheduler_wpPostStore::SCHEDULE_META_KEY, + ), + array( '%s' ), + array( '%d', '%s' ) + ); + // phpcs:enable WordPress.DB.SlowDBQuery.slow_db_query_meta_value, WordPress.DB.SlowDBQuery.slow_db_query_meta_key, WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize + clean_post_cache( $action_id ); + wp_cache_delete( $action_id, 'post_meta' ); + + try { + $store->fetch_action( $action_id ); + } catch ( Exception $e ) { + unset( $e ); // Rejecting the action is an acceptable outcome; we only assert on the side effect. + } + + $this->assertFalse( + ActionScheduler_Test_Evil_Gadget::$fired, + 'The post-based store instantiated an arbitrary class from a schedule blob.' + ); + } + + /** + * The post-based store must still round-trip legitimate schedules after hardening. + */ + public function test_wp_post_store_round_trips_valid_schedule() { + $time = as_get_datetime_object( '2026-05-06 07:08:09' ); + $store = new ActionScheduler_wpPostStore(); + $action = new ActionScheduler_Action( ActionScheduler_Callbacks::HOOK_WITH_CALLBACK, array(), new ActionScheduler_SimpleSchedule( $time ) ); + $action_id = $store->save_action( $action ); + + $fetched = $store->fetch_action( $action_id ); + $schedule = $fetched->get_schedule(); + + $this->assertInstanceOf( 'ActionScheduler_SimpleSchedule', $schedule ); + $this->assertEquals( $time->getTimestamp(), $schedule->get_date()->getTimestamp() ); + } + + public function tearDown(): void { + // These tests register filters/actions on the deserializer's hooks; clear them so nothing leaks + // into later tests running in the same process. + remove_all_filters( 'action_scheduler_allowed_nested_schedule_classes' ); + remove_all_filters( 'action_scheduler_enforce_schedule_allowed_classes' ); + remove_all_actions( 'action_scheduler_unexpected_schedule_class' ); + remove_all_actions( 'action_scheduler_failed_fetch_action' ); + parent::tearDown(); + } + + /** + * A serialized blob for a valid schedule (ActionScheduler_IntervalSchedule) that smuggles a gadget + * in as its (protected) recurrence property. Hand-crafted because __sleep() would otherwise drop a + * non-whitelisted property. + * + * @return string + */ + private function nested_gadget_blob() { + $nul = chr( 0 ); + $prop_ts = $nul . '*' . $nul . 'scheduled_timestamp'; // Protected property marker. + $prop_rec = $nul . '*' . $nul . 'recurrence'; // Protected property marker. + $gadget = 'ActionScheduler_Test_Evil_Gadget'; + $container = 'ActionScheduler_IntervalSchedule'; + + return 'O:' . strlen( $container ) . ':"' . $container . '":2:{' + . 's:' . strlen( $prop_ts ) . ':"' . $prop_ts . '";i:' . ( time() + HOUR_IN_SECONDS ) . ';' + . 's:' . strlen( $prop_rec ) . ':"' . $prop_rec . '";' + . 'O:' . strlen( $gadget ) . ':"' . $gadget . '":0:{}' + . '}'; + } + + /** + * Shadow mode ("report only") must NEVER let a bare top-level gadget be instantiated. Only an + * unexpected *nested* class inside an otherwise-valid schedule may pass through in shadow mode. + */ + public function test_shadow_mode_still_blocks_top_level_gadget() { + add_filter( 'action_scheduler_enforce_schedule_allowed_classes', '__return_false' ); + + $blob = serialize( new ActionScheduler_Test_Evil_Gadget() ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize + $result = ActionScheduler_ScheduleDeserializer::unserialize( $blob ); + + $this->assertFalse( $result, 'A top-level gadget was not rejected in shadow mode.' ); + $this->assertFalse( + ActionScheduler_Test_Evil_Gadget::$fired, + 'A top-level gadget was instantiated in shadow mode.' + ); + } + + /** + * Shadow mode DOES let an unexpected nested class through: this is its documented purpose (keep an + * otherwise-legitimate action working while an allow-list is being tuned). The tradeoff — and the + * reason enforcement is the default — is that the nested class is instantiated. + */ + public function test_shadow_mode_allows_unexpected_nested_class_through() { + add_filter( 'action_scheduler_enforce_schedule_allowed_classes', '__return_false' ); + + $result = ActionScheduler_ScheduleDeserializer::unserialize( $this->nested_gadget_blob() ); + + $this->assertInstanceOf( + 'ActionScheduler_IntervalSchedule', + $result, + 'Shadow mode did not pass a valid schedule with an unexpected nested class through.' + ); + $this->assertTrue( + ActionScheduler_Test_Evil_Gadget::$fired, + 'Shadow mode is report-only for nested classes, so the nested class is expected to run.' + ); + } + + /** + * A rejection must surface the action_scheduler_unexpected_schedule_class action for observability, + * reporting the offending class, the outer class, and that the blob was rejected. + */ + public function test_unexpected_class_action_fires_on_rejection() { + $captured = array(); + add_action( + 'action_scheduler_unexpected_schedule_class', + // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed + function ( $offending, $outer, $unexpected, $rejected ) use ( &$captured ) { + $captured = compact( 'offending', 'outer', 'unexpected', 'rejected' ); + }, + 10, + 4 + ); + + $blob = serialize( new ActionScheduler_Test_Evil_Gadget() ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize + $result = ActionScheduler_ScheduleDeserializer::unserialize( $blob ); + + $this->assertFalse( $result ); + $this->assertSame( 'ActionScheduler_Test_Evil_Gadget', $captured['offending'] ); + $this->assertSame( 'ActionScheduler_Test_Evil_Gadget', $captured['outer'] ); + $this->assertTrue( $captured['rejected'] ); + } + + /** + * The action_scheduler_allowed_nested_schedule_classes filter must be honored, and — because it is + * resolved per call rather than cached for the process — a class allow-listed only for the second + * call is rejected on the first and accepted on the second. + */ + public function test_nested_allow_list_filter_is_honored_per_call() { + $blob = serialize( new ActionScheduler_Test_Custom_Schedule_With_Property( time() + DAY_IN_SECONDS, new ActionScheduler_Test_Schedule_Helper( 42 ) ) ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize + + // First call, no filter: the helper class is unexpected, so the blob is rejected. + $this->assertFalse( + ActionScheduler_ScheduleDeserializer::unserialize( $blob ), + 'An un-allow-listed nested support class should be rejected by default.' + ); + + // Second call, with the helper allow-listed via the filter: the blob is accepted. This also + // proves the first, filter-less call did not cache a stale allow-list for the process. + add_filter( + 'action_scheduler_allowed_nested_schedule_classes', + function ( $classes ) { + $classes[] = 'ActionScheduler_Test_Schedule_Helper'; + return $classes; + } + ); + + $this->assertInstanceOf( + 'ActionScheduler_Test_Custom_Schedule_With_Property', + ActionScheduler_ScheduleDeserializer::unserialize( $blob ), + 'A nested class allow-listed via the filter should be accepted.' + ); + } + + /** + * A third party schedule that keeps built-in date/time value objects (DateTime, DateTimeImmutable, + * DateTimeZone, DateInterval) as properties must round-trip with no configuration — those classes + * are on the default nested allow-list. + */ + public function test_third_party_schedule_nesting_datetime_objects_round_trips() { + $timestamp = time() + DAY_IN_SECONDS; + $blob = serialize( new ActionScheduler_Test_Custom_Schedule_With_Property( $timestamp ) ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize + + $result = ActionScheduler_ScheduleDeserializer::unserialize( $blob ); + + $this->assertInstanceOf( 'ActionScheduler_Test_Custom_Schedule_With_Property', $result ); + $this->assertEquals( $timestamp, $result->next()->getTimestamp() ); + } + + /** + * A pathologically deep object graph must be rejected rather than walked into a stack overflow. + * (Cyclic graphs are covered separately; this covers sheer depth beyond MAX_GRAPH_DEPTH.) + */ + public function test_excessively_deep_graph_is_rejected() { + // A valid (third party) schedule class as the outer object so the top-level gate passes, wrapping + // a scalar buried in ~150 nested arrays — well beyond the depth we are willing to vet. + $class = 'ActionScheduler_Test_Custom_Schedule_With_Property'; + $deep = 'i:1;'; + for ( $i = 0; $i < 150; $i++ ) { + $deep = 'a:1:{i:0;' . $deep . '}'; + } + $blob = 'O:' . strlen( $class ) . ':"' . $class . '":1:{s:4:"deep";' . $deep . '}'; + + $this->assertFalse( + ActionScheduler_ScheduleDeserializer::unserialize( $blob ), + 'A graph deeper than the vetting bound should be rejected.' + ); + } + + /** + * The deserializer instance is reusable: invoking it for one blob must not leak walk state into the + * next. A rejected blob followed by a clean one must not taint the clean result. + */ + public function test_deserializer_instance_is_reusable() { + $deserializer = new ActionScheduler_ScheduleDeserializer( + array( 'ActionScheduler_Test_Custom_Schedule' ), + array() + ); + + // First: a blob nesting a gadget (rejected, populating internal offender/seen state). + $this->assertFalse( $deserializer( $this->nested_gadget_blob() ) ); + + // Then: a clean third party schedule must still deserialize correctly. + $clean = serialize( new ActionScheduler_Test_Custom_Schedule( time() + HOUR_IN_SECONDS ) ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize + $this->assertInstanceOf( + 'ActionScheduler_Test_Custom_Schedule', + $deserializer( $clean ), + 'Reusing a deserializer instance leaked state from a prior blob.' + ); + } + + /** + * The constructor's nested allow-list is authoritative: a class it lists is accepted when nested, + * and the same blob is rejected when it is not listed. + */ + public function test_constructor_nested_allow_list_is_used() { + $blob = serialize( new ActionScheduler_Test_Custom_Schedule_With_Property( time() + DAY_IN_SECONDS, new ActionScheduler_Test_Schedule_Helper( 7 ) ) ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize + + $without = new ActionScheduler_ScheduleDeserializer( array(), array() ); + $this->assertFalse( $without( $blob ), 'Nested support classes absent from the allow-list must be rejected.' ); + + $with = new ActionScheduler_ScheduleDeserializer( + array(), + array( 'DateTime', 'DateTimeImmutable', 'DateTimeZone', 'DateInterval', 'ActionScheduler_Test_Schedule_Helper' ) + ); + $this->assertInstanceOf( + 'ActionScheduler_Test_Custom_Schedule_With_Property', + $with( $blob ), + 'A nested class present in the injected allow-list must be accepted.' + ); + } + + /** + * The post-based store reads the raw schedule meta. A non-serialized meta value cannot be a schedule + * object, so it is left for validate_schedule() to reject rather than being deserialized — which + * fetch_action() surfaces as a failed fetch and a null action, not a usable schedule. + */ + public function test_wp_post_store_rejects_non_serialized_schedule_meta() { + $store = new ActionScheduler_wpPostStore(); + $action = new ActionScheduler_Action( ActionScheduler_Callbacks::HOOK_WITH_CALLBACK, array(), new ActionScheduler_SimpleSchedule( as_get_datetime_object() ) ); + $action_id = $store->save_action( $action ); + + global $wpdb; + // phpcs:disable WordPress.DB.SlowDBQuery.slow_db_query_meta_value, WordPress.DB.SlowDBQuery.slow_db_query_meta_key + $wpdb->update( + $wpdb->postmeta, + array( 'meta_value' => 'this-is-not-serialized' ), + array( + 'post_id' => $action_id, + 'meta_key' => ActionScheduler_wpPostStore::SCHEDULE_META_KEY, + ), + array( '%s' ), + array( '%d', '%s' ) + ); + // phpcs:enable WordPress.DB.SlowDBQuery.slow_db_query_meta_value, WordPress.DB.SlowDBQuery.slow_db_query_meta_key + clean_post_cache( $action_id ); + wp_cache_delete( $action_id, 'post_meta' ); + + $failed = false; + add_action( + 'action_scheduler_failed_fetch_action', + function () use ( &$failed ) { + $failed = true; + } + ); + + $fetched = $store->fetch_action( $action_id ); + + $this->assertTrue( $failed, 'A non-serialized schedule meta value was not treated as an invalid action.' ); + $this->assertInstanceOf( 'ActionScheduler_NullAction', $fetched ); + } + + /** + * as_get_datetime_object() returns an ActionScheduler_DateTime (a DateTime subclass), so a third + * party schedule that stores the result of that idiomatic helper as a property nests one rather than + * a plain DateTime. It must be on the default nested allow-list, or such a schedule would be rejected. + */ + public function test_third_party_schedule_nesting_action_scheduler_datetime_round_trips() { + // A third party schedule (protected $timestamp) holding an ActionScheduler_DateTime instead of an + // int. Hand-crafted so the nested object is genuinely an ActionScheduler_DateTime. + $nul = chr( 0 ); + $prop = $nul . '*' . $nul . 'timestamp'; + $class = 'ActionScheduler_Test_Custom_Schedule'; + $date = serialize( as_get_datetime_object( '2026-01-02 03:04:05' ) ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize + $blob = 'O:' . strlen( $class ) . ':"' . $class . '":1:{s:' . strlen( $prop ) . ':"' . $prop . '";' . $date . '}'; + + $result = ActionScheduler_ScheduleDeserializer::unserialize( $blob ); + + $this->assertInstanceOf( 'ActionScheduler_Test_Custom_Schedule', $result ); + + $property = new ReflectionProperty( 'ActionScheduler_Test_Custom_Schedule', 'timestamp' ); + $property->setAccessible( true ); + $this->assertInstanceOf( 'ActionScheduler_DateTime', $property->getValue( $result ) ); + } + + /** + * A gadget buried inside a valid third party schedule wrapper must be rejected without ever being + * instantiated. Unlike the core-schedule wrapper case, the outer object is itself a placeholder in + * the safe parse, so this exercises the walk recursing into a placeholder to find a nested one. + */ + public function test_gadget_nested_in_third_party_schedule_is_rejected_without_instantiation() { + $nul = chr( 0 ); + $prop = $nul . '*' . $nul . 'timestamp'; + $class = 'ActionScheduler_Test_Custom_Schedule'; // Valid third party schedule → placeholder in the safe parse. + $gadget = serialize( new ActionScheduler_Test_Evil_Gadget() ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize + $blob = 'O:' . strlen( $class ) . ':"' . $class . '":1:{s:' . strlen( $prop ) . ':"' . $prop . '";' . $gadget . '}'; + + $result = ActionScheduler_ScheduleDeserializer::unserialize( $blob ); + + $this->assertFalse( $result ); + $this->assertFalse( + ActionScheduler_Test_Evil_Gadget::$fired, + 'A gadget nested in a third party schedule was instantiated.' + ); + } + + /** + * A tampered blob can make an otherwise-trusted schedule class throw during deserialization — for + * example, a valid schedule whose `scheduled_timestamp` scalar has been replaced with an object, + * which the schedule's `__wakeup()` then feeds to a `DateTime` constructor. That surfaces as a + * `TypeError` (an `Error`, which `@` does not suppress and a `catch ( Exception )` does not catch). + * The deserializer must absorb it and report corrupt data, not let it escape and fatal the read. + */ + public function test_tampered_blob_that_makes_a_trusted_class_throw_is_reported_as_corrupt() { + $nul = chr( 0 ); + $prop = $nul . '*' . $nul . 'scheduled_timestamp'; // Protected property marker. + $class = 'ActionScheduler_IntervalSchedule'; // Trusted → instantiated (and woken) in the safe parse. + $evil = 'O:12:"Totally_Fake":0:{}'; // An object where a timestamp is expected. + $blob = 'O:' . strlen( $class ) . ':"' . $class . '":1:{s:' . strlen( $prop ) . ':"' . $prop . '";' . $evil . '}'; + + // Without the guard this call throws an uncaught TypeError and this test errors instead of passing. + $this->assertFalse( + ActionScheduler_ScheduleDeserializer::unserialize( $blob ), + 'A tampered blob that makes a trusted class throw should be reported as corrupt (false).' + ); + + // And the store must surface it as a handled (null) action rather than fataling the read. + $action_id = $this->store_action_with_raw_schedule( $blob ); + $store = new ActionScheduler_DBStore(); + $fetched = null; + try { + $fetched = $store->fetch_action( $action_id ); + } catch ( Exception $e ) { + unset( $e ); // A handled InvalidActionException is acceptable; an Error would have fataled above. + } + $this->assertNotNull( $fetched, 'Fetching a tampered action fataled instead of being handled.' ); + } +}