[wp-trac] [WordPress Trac] #58221: Cast (int) $cron->time - time() to prevent String - int error

[WordPress Trac]

#58221: Cast (int) $cron->time - time() to prevent String - int error
----------------------------+------------------------------
 Reporter:  toddlahman      |       Owner:  (none)
     Type:  defect (bug)    |      Status:  closed
 Priority:  normal          |   Milestone:  Awaiting Review
Component:  Site Health     |     Version:  6.2
 Severity:  critical        |  Resolution:  maybelater
 Keywords:  has-patch php8  |     Focuses:
----------------------------+------------------------------
Changes (by toddlahman):

 * status:  new => closed
 * resolution:   => maybelater


Comment:

 Upon further investigation I found it was a plugin, or some other code,
 that directly altered the array stored in cron in the options table. I
 rarely use any plugins from anywhere that aren't written for WooCommerce
 or by seasoned developers. After modifications to the WP Control plugin I
 was able to visualize the issue
 [https://www.dropbox.com/s/9j5d82crqiw6kum/Screen%20Shot%202023-04-29%20at%204.51.38%20PM.png?dl=0].
 However the cron var_dump using
 {{{
 _get_cron_array()
 }}}
 [ https://www.dropbox.com/s/ns8tcrhlkzvbm5k/cron_dump.txt?dl=0] revealed a
 bigger issue. On line 599 the top level array key is not an integer, but a
 string 'wp_batch_split_terms'. The cron API code does prevent anything
 other than an integer from being used as the epoch/unix timestamp, however
 by appending the cron array in the options table, a plugin, or some other
 code, was able to bypass the code safeguards. I was able to remove the
 entries with the following code:


 {{{
                 $cron = get_option( 'cron' );
                 update_option( 'old_cron', $cron );
                 unset( $cron['wp_batch_split_terms'] );
                 update_option( 'cron', $cron );
 }}}


 I could not use a plugin to delete these entries because they typically
 rely on the cron API, as they should.

 For the end user who never sees the cron entries, this is a critical
 issue, since their site will not behave as expected, such as the health
 report not being visible due to a fatal error. There is nothing in the
 code that validates the timestamp as an integer when the cron array is
 retrieved from the options table, and no other values from the cron array
 are validated for their data type. This led to fatal errors on my site.
 PHP itself is not type safe, and in this instance casting a String to an
 integer may lead to a zero value, which isn't much help. Cron is used
 routinely. I have set
 {{{
 define( 'DISABLE_WP_CRON', true );
 }}}
 , but most sites do not. Relying on code to prevent incorrect data types
 from being used is not sufficient because the data can be directly
 manipulated in the options database table. It would be better to create a
 custom cron database table that can enforce a BIGINT timestamp column, and
 other appropriate data types, to insure this type of fatal error cannot
 happen in the future. Migrating the current cron array of data would be
 beneficial to those who have incorrect data for cron entries currently
 that would not be allowed in the new cron table due to the column data
 type enforcement.

 To fix this issue, my suggestion is to create a custom cron database
 table. This would have the side effect of making sites perform faster, in
 addition to insuring the data had the correct data types.

 If no one is available to make this addition, I could submit this as a
 core contribution in the fall.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/58221#comment:5>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform