Make WordPress Core

Opened 3 months ago

Last modified 3 weeks ago

#64404 accepted defect (bug)

Uncaught DivisionByZeroError: Modulo by zero

Reported by: kaushiksomaiya's profile kaushiksomaiya Owned by: westonruter's profile westonruter
Milestone: 7.0 Priority: normal
Severity: normal Version:
Component: Cron API Keywords: has-patch
Focuses: Cc:

Description

Hello team,

It seems there's a possibility of plugins or custom code inadvertently inserting boolean value in custom schedule intervals. When that happens - Cron breaks badly even after removal of the custom code/plugin declaring it. This is because there's an attempt to reschedule the event before executing it.. and probably if ( 0 === $interval ) { doesn't evaluate true for the bool value.

To Replicate:

// 1. Add a broken schedule
add_filter( 'cron_schedules', function( $schedules ) {

    // INTENTIONALLY BROKEN
    $schedules['kscronbreaker_cron_schedule'] = array(
        'interval' => false,          // ❌ Wrong: must be integer > 0
        'display'  => 'Broken Cron Schedule',
    );

    return $schedules;
} );

// 2. Try to schedule the event
add_action( 'init', function() {

    if ( ! wp_next_scheduled( 'kscronbreaker_broken_cron_hook' ) ) {
        wp_schedule_event(
            time() + 10,
            'kscronbreaker_cron_schedule',
            'kscronbreaker_broken_cron_hook'
        );
    }
} );

// 3. Cron handler (never runs)
add_action( 'kscronbreaker_broken_cron_hook', function() {
    error_log( 'KS Cron Breaker executed — THIS SHOULD NEVER APPEAR' );
} );

Then check the server's fatal error logs

Uncaught DivisionByZeroError: Modulo by zero in /wp-5351423/wp-includes/cron.php:436

Change History (10)

This ticket was mentioned in PR #10619 on WordPress/wordpress-develop by @westonruter.


3 months ago
#1

  • Keywords has-patch added

#2 @westonruter
3 months ago

  • Milestone changed from Awaiting Review to 7.0

This seems like something which would be worthwhile to fix, given that there is already is error checking for an invalid interval. Here's a quick proposal to fix: https://github.com/WordPress/wordpress-develop/pull/10619

#3 @westonruter
3 months ago

  • Owner set to westonruter
  • Status changed from new to accepted

#4 @soyebsalar01
3 months ago

Tested PR 10619 after rebuilding from src.

After using the custom cron schedule with an invalid interval (false) no longer causes a DevisionByZeroError. So the cron execute the event once and then fails to reschedule gracefully with invalid_schedule.

Patch works as expected.

@johnbillion commented on PR #10619:


3 months ago
#5

There's a failing test:

1) Tests_Cron::test_invalid_recurrence_for_event_returns_error
sprintf(): Argument number must be greater than zero

/var/www/src/wp-includes/cron.php:428
/var/www/tests/phpunit/tests/cron.php:856
phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:106
/var/www/vendor/bin/phpunit:118

#6 @vapvarun
3 weeks ago

Ran into this on trunk (9e4cb58243), PHP 8.2.29.

Makes sense -- 0 === $interval on line 447 is strict, so false gets past it and hits the modulo on 463.

Tested PR #10619 -- it fixes the crash, but there's a failing test:

Tests_Cron::test_invalid_recurrence_for_event_returns_error
sprintf(): Argument number must be greater than zero
wp-includes/cron.php:428

One thought -- instead of 0 === $interval, widen the check to catch false, 0, and negative values:

In wp_reschedule_event() around line 447:

if ( ! is_numeric( $interval ) || $interval <= 0 ) {
    if ( $wp_error ) {
        if ( ! isset( $schedules[ $recurrence ] ) ) {
            return new WP_Error( 'invalid_schedule', __( 'Event schedule does not exist.' ) );
        }
        return new WP_Error( 'invalid_interval', __( 'Event schedule has an invalid interval.' ) );
    }
    return false;
}

And in wp_schedule_event() around line 283, validate the interval before saving:

$interval = $schedules[ $recurrence ]['interval'];
if ( ! is_numeric( $interval ) || $interval <= 0 ) {
    if ( $wp_error ) {
        return new WP_Error( 'invalid_interval', __( 'Event schedule has an invalid interval.' ) );
    }
    return false;
}

This way bad intervals get caught at schedule time too, not just on reschedule. Cron tests pass locally (75/75). Happy to open a PR if the approach looks right.

#7 @westonruter
3 weeks ago

@vapvarun There's a failing test? Not when the tests last ran. I just merged the latest from trunk onto the branch for that PR since it is a couple months stale. Did you merge from trunk to get that test failure?

#8 @vapvarun
3 weeks ago

@westonruter Thanks for the quick follow-up and for merging trunk into the PR. Re-tested PR #10619 on d1c3ba618d, PHP 8.2.29 — all 74 cron tests pass cleanly now. The sprintf() issue from comment:5 appears to have been resolved by the trunk merge. My earlier testing was before that merge, so the failure was real at the time but is no longer an issue.

The updated PR looks great. Nice touch validating $event->interval after the schedule_event filter rather than before event creation — that also catches cases where a filter might set a bad interval.

Two suggestions on the test coverage:

  1. The current test only covers negative intervals. The original bug was 'interval' => false - might be worth adding that as a test case alongside zero:
$schedules['bad_false'] = array(
    'interval' => false,
    'display'  => 'Broken interval (false)',
);
$schedules['bad_zero'] = array(
    'interval' => 0,
    'display'  => 'Broken interval (zero)',
);
  1. No test for the $wp_error = false path (should return false instead of WP_Error).

This ticket was mentioned in Slack in #core-test by ozgur_sar. View the logs.


3 weeks ago

#10 @huzaifaalmesbah
3 weeks ago

Test Report

Patch Tested: PR #10619

Environment

  • WordPress: 7.0-beta1-61709-src
  • PHP: 8.2.29
  • Server: nginx/1.29.5
  • Database: mysqli (Server: 8.4.8 / Client: mysqlnd 8.2.29)
  • Browser: Chrome 145.0.0.0
  • OS: macOS
  • Theme: Twenty Eleven 5.0
  • MU Plugins:
    • test-64404.php
  • Plugins:
    • Test Reports 1.2.1

Steps taken

  1. Create a file named test-64404.php in the /wp-content/mu-plugins/ directory.
  2. Paste the provided MU Plugin Code from the Support Content section below into the file. This code intentionally registers a broken cron schedule ('interval' => false).
  3. Run the following WP-CLI command to trigger the init action and properly schedule the broken cron event:
    wp eval 'echo "init triggered";'
    
  4. List the scheduled events to verify the broken hook is in the queue:
    wp cron event list
    
  5. Testing trunk (Before Patch): While on trunk, run the broken cron event manually:
    wp cron event run kscronbreaker_broken_cron_hook
    
  6. The process crashes and a fatal DivisionByZeroError is returned:
    Fatal error: Uncaught DivisionByZeroError: Modulo by zero in /var/www/src/wp-includes/cron.php:463
    Stack trace:
    #0 phar:///usr/local/bin/wp/vendor/wp-cli/cron-command/src/Cron_Event_Command.php(352): wp_reschedule_event(1771947435, 'kscronbreaker_c...', 'kscronbreaker_b...', Array)
    #1 phar:///usr/local/bin/wp/vendor/wp-cli/cron-command/src/Cron_Event_Command.php(238): Cron_Event_Command::run_event(Object(stdClass))
    ...
    Error: There has been a critical error on this website.
    
  7. Testing PR #10619 (After Patch): Apply the patch from PR 10619.
  8. Run the exact same WP-CLI command to execute the broken event:
    wp cron event run kscronbreaker_broken_cron_hook
    
  9. The event gracefully skips rescheduling itself without crashing PHP. The command successfully finishes executing:
    Executed the cron event 'kscronbreaker_broken_cron_hook' in 0.04s.
    Success: Executed a total of 1 cron event.
    
  10. Patch is solving the problem / works as expected

Expected Results

When a scheduled cron event has an invalid interval (such as a boolean false instead of an integer), the system should gracefully fail to reschedule the event. It should not cause a fatal DivisionByZeroError that breaks the entire cron execution process.

Additional Notes

  • The patch effectively guards against division/modulo by zero by validating that the interval is functionally greater than 0 before performing the calculation.

Support Content

1. MU Plugin Code (wp-content/mu-plugins/test-64404.php)

<?php
// 1. Add a broken schedule
add_filter( 'cron_schedules', function( $schedules ) {
        // INTENTIONALLY BROKEN
        $schedules['kscronbreaker_cron_schedule'] = array(
                'interval' => false, // ❌ Wrong: must be integer > 0
                'display'  => 'Broken Cron Schedule',
        );
        return $schedules;
} );

// 2. Try to schedule the event
add_action( 'init', function() {
        if ( ! wp_next_scheduled( 'kscronbreaker_broken_cron_hook' ) ) {
                wp_schedule_event( time() + 10, 'kscronbreaker_cron_schedule', 'kscronbreaker_broken_cron_hook' );
        }
} );

// 3. Cron handler (never runs on trunk)
add_action( 'kscronbreaker_broken_cron_hook', function() {
        error_log( 'KS Cron Breaker executed!' );
} );
Note: See TracTickets for help on using tickets.