Inconsistent strtotime handling of relative dates from user data

Xon

Well-known member
Affected version
2.2.7 Patch 1
In a number of places strtotime is used to parse something like "+$value $unit" from user-input to generate a datetime stamp from a relative time structure.

The problem is this is somewhat inconsistent in a number of places. Most of the time 0 means permanent, but some places have a check against pow(2,32) - 1.

If a bad value is passed in (likely due to editing html or bad templating), then strtotime return false rather than raise an error and silently fails to being 'forever'.

This affects at least;
  • reply-bans
  • warnings
  • polls
  • warning actions
  • user upgrades
XF\Searcher\AbstractSearcher::convertRelativeTimeToCutoff exists, but likely needs to be a utility function which can optionally push the error onto a linked entity + key. Note; probably should just try passing the unit to strtotime[B][/B] rather than having a fixed list, since seconds/minutes are valid combinations.

For example, when issuing a warning;
PHP:
if ($input['expiry_enable'])
{
   $expiry = strtotime('+' . $input['expiry_value'] . ' ' . $input['expiry_unit']);
}
else
{
   $expiry = 0;
}

Perhaps something like this (but in a utility function and used everywhere else);
PHP:
$expiry = @\strtotime('+' . $input['expiry_value'] . ' ' . $input['expiry_unit']);
if ($expiry === false)
{
   throw $this->exception($this->error(\XF::phrase('invalid_offset_date', [
        'expiryLength' => $expiryLength,
        'expiryUnit'   => $expiryUnit,
    ]), 'expiry_date'));
}
$expiry = (int)$expiry;
if ($expiry >= pow(2, 32) - 1)
{
    $expiry = 0;
}
 
Top