Sanitize return value of 'nonce_life' filters in wp_nonce_tick() to avoid DivisionByZeroError

[marian1]

wp_nonce_tick() uses the return value of a nonce_life filter in a division. If that filter returns zero, or any other value that evaluates to zero in a numeric context, wp_nonce_tick() will perform a division by zero. This results in a DivisionByZeroError on PHP 8, or a warning with a return value of float(INF) on PHP 7.

<?php
$nonce_life = apply_filters( 'nonce_life', DAY_IN_SECONDS, $action );
return ceil( time() / ( $nonce_life / 2 ) );

Values received from filters should be validated and sanitised before use. In this case, wp_nonce_tick() should handle invalid values gracefully by falling back to the default lifespan when the filtered value is not a positive integer.

For example:

<?php
$nonce_life = (int) apply_filters( 'nonce_life', DAY_IN_SECONDS, $action );
if ( $nonce_life <= 0 ) {
        $nonce_life = DAY_IN_SECONDS;
}

This would prevent fatal errors when a filter (mistakenly) returns a value that evaluates to zero.

[knutsp]

Why should the fatal error be prevented when it's the result of a developer mistake? How would the developer then know there is such a mistake?

IMO, if anything and actually common and plain misunderstanding, this is a candidate for "doing it wrong".

[johnbillion]

Thanks for the report, but this is a valid situation to allow PHP to trigger an error. Working around it just papers over the developer error.

Cheers!

[marian1]

$user === $developer is not always true

A user installing, for example, a plugin that registers such a filter callback (and with auto-updates enabled, this can happen without any action on their part) is not necessarily the developer of that plugin, nor the person contacted about the error. This situation could be handled much more gracefully by simply validating filters and logging any errors--particularly as a reasonable default value exists.

The responsible developer could still be contacted by someone reviewing the logs, without causing panic for the user or leaving the site non-functional. At the end of the day, WordPress is not only used by people who do not know how to deal with, or even read, such an error, but is also marketed to them.

WordPress as a safeguard, not a bugfixer

I am aware that this issue would be caused by a plugin or theme developer rather than by WordPress itself, which is why I have classified it as an enhancement rather than a bug.

Aside from the unvalidated filters across the entire codebase, my impression was that WordPress’ approach is more of a "better safe than sorry" policy. Perhaps my impression of WordPress’ role is mistaken--namely, that as such a heavily used platform with a massive number of themes and plugins, it also sees itself as a safeguard for users against developer errors. I read practices such as type-checking function arguments and validating or sanitising some filters as an indication of that.

Skip reporting similar issues?

That said, I can live with WordPress not taking on this role. Am I correct in assuming that I can skip reporting any other issues of the same nature?

Apologies if this has already been discussed--for someone not involved in WordPress core development, it can be a little overwhelming to figure where to find such information.