Make WordPress Core

Opened 9 months ago

Last modified 6 weeks ago

#64404 accepted defect (bug)

Uncaught DivisionByZeroError: Modulo by zero

Reported by: kaushiksomaiya Owned by: westonruter
Priority: normal Milestone: 7.2
Component: Cron API Version:
Severity: normal Keywords: has-patch has-unit-tests changes-requested
Cc: Focuses:

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 (16)

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


9 months ago
#1

  • Keywords has-patch added

#2 @westonruter
9 months ago

  • Milestone Awaiting Review7.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
9 months ago

  • Owner set to westonruter
  • Status newaccepted

#4 @soyebsalar01
9 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:


9 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
7 months 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
7 months 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
7 months 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.


7 months ago

#10 @huzaifaalmesbah
7 months 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!' );
} );

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


6 months ago

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


6 months ago

#13 @audrasjb
6 months ago

  • Milestone 7.07.1

As per today's 7.0 pre-RC1 bug scrub:
As we're a few hours from 7.0 RC1, I'm moving this ticket to milestone 7.1 to give it more time for testing.

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


2 months ago
#14

  • Keywords has-unit-tests added

## Summary

A cron schedule registered with a fractional interval (for example 'interval' => 0.5) causes a fatal DivisionByZeroError when the event is rescheduled.

wp_reschedule_event() guards against a bad interval with if ( 0 == $interval ). That loose comparison catches false, 0, and null, but not a fractional value between 0 and 1: 0 == 0.5 is false, so execution continues to:

$timestamp = $now + ( $interval - ( ( $now - $timestamp ) % $interval ) );

PHP's % operator casts 0.5 to the integer 0, so this throws DivisionByZeroError (preceded by an Implicit conversion from float ... to int loses precision notice on PHP 8.1+).

### Reproduction

add_filter( 'cron_schedules', function ( $schedules ) {
    $schedules['fractional'] = array( 'interval' => 0.5, 'display' => 'Fractional' );
    return $schedules;
} );

// Rescheduling with a past timestamp hits the modulo branch:
wp_reschedule_event( time() - HOUR_IN_SECONDS, 'fractional', 'hook' );
// Fatal error: Uncaught DivisionByZeroError: Modulo by zero in wp-includes/cron.php:433

## Fix

Widen the guard so the interval is validated the same way the arithmetic truncates it:

if ( ! is_numeric( $interval ) || (int) $interval <= 0 ) {

This rejects fractional (0.5), zero, negative, and non-numeric intervals before they reach the modulo, so the event fails to reschedule gracefully (returning false, or a WP_Error when $wp_error is set) instead of throwing a fatal error.

## Testing

Added test_fractional_schedule_interval_does_not_throw, covering both the $wp_error and default return paths.

  • Before the fix, the test errors at cron.php:433 (Implicit conversion from float 0.5 to int loses precision / DivisionByZeroError).
  • After the fix, it passes.

Full cron group is green:

./vendor/bin/phpunit --group cron
OK (70 tests, 232 assertions)

## Relationship to PR #10619

This is a focused, standalone fix for the fatal error. PR #10619 takes a broader approach to interval validation; during its review, @rbcorrales pointed out that fractional intervals between 0 and 1 still slip through is_numeric( $interval ) && $interval > 0 because the modulo truncates them to 0. That case was still unaddressed, so this PR fixes it directly and adds explicit regression coverage. Whichever approach lands, the fractional case and its test should be included.

Props rbcorrales.

## Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 4.8
Used for: Reproducing the fractional-interval crash, implementing the guard, and writing the regression test. All changes were reviewed by me.

#15 @wildworks
6 weeks ago

  • Keywords changes-requested added

PR 10619 appears to need updates based on feedback. Additionally, we might be able to reference the implementation in PR 12547. In any case, if this doesn't make it into the upcoming RC1, we will punt it to 7.2.

#16 @westonruter
6 weeks ago

  • Milestone 7.17.2
Note: See TracTickets for help on using tickets.