Invalid value range in ext/date/lib/interval.c

[YuanchengJiang]

Description

The following code:

<?php
foreach ([PHP_INT_MIN, PHP_INT_MAX] as $extreme) {
$i = 64;
while ($i-- > 1) {
$d = new DateTime('@' . ($extreme >> $i));
}
}
$script1_dataflow = $d;
$datetime3 = new DateTime('13-03-02');
$interval = $script1_dataflow->diff($datetime3);

Resulted in this output:

/php-src/ext/date/lib/interval.c:204:10: runtime error: 5.3376e+13 is outside the range of representable values of type 'int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /php-src/ext/date/lib/interval.c:204:10 in

PHP Version

PHP 8.4.0-dev

Operating System

ubuntu 22.04

[cmb69]

This is apparently about this line:

php-src/ext/date/lib/interval.c

Line 204 in a578c27

days = fabs(floor(one->sse - two->sse) / 86400);

That floor() call doesn't appear to make sense; is it supposed to be

		days = fabs(floor((one->sse - two->sse) / 86400));

instead?

That wouldn't solve the issue at hand, though. This is caused because an int can't store such large differences, e.g.

$d1 = new DateTime("@" . PHP_INT_MAX);
$d2 = new DateTime("@0");
$interval = $d1->diff($d2);
var_dump($interval->days);

outputs for me int(579025220) which appears to be wrong.

[YuanchengJiang]

it happens again:

<?php
$nowTime = new DateTime();
$dateTime = new DateTime();
$dateTime->setTimestamp(PHP_INT_MAX);
$dateTime->setTimezone(new DateTimeZone('America/New_York'));
echo $dateTime->diff($nowTime)->format('%R %a %H %I %S'), "\n";

[cmb69]

@YuanchengJiang, the fix is only a stop-gap measure, since it tries to pass --fwrapv to the compiler; if the compiler does not support this option (such as clang-cl on Windows), there is still undefined behavior. Please check if -fwrapv works for your compiler (should work for GCC and Clang).

By the way, the new test script gives for me:

ext\date\lib\parse_posix.c:493:22: runtime error: signed integer overflow: 86400 * 106751991167328 cannot be represented in type 'timelib_sll' (aka 'long long')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ext\date\lib\parse_posix.c:493:22
ext\date\lib\interval.c:204:10: runtime error: 1.06752e+14 is outside the range of representable values of type 'int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ext\date\lib\interval.c:204:10

So there is another (possibly Windows specific) issue in parse_posix.c.

Anyhow, a clean solution would require invasive changes to timelib; at least it's a lot of work.

[cmb69]

Ah, completely forgot that the UB in interval.c has already been resolved upstream (derickr/timelib#162); that will be merged into php-src eventually.

[cmb69]

To prevent the signed integer overflow in timelib_ts_at_start_of_year(), we could cap to range, something like:

 ext/date/lib/parse_posix.c | 33 ++++++++++++++++++++++++++++-----
 ext/date/lib/timelib.h     |  4 ++++
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/ext/date/lib/parse_posix.c b/ext/date/lib/parse_posix.c
index 204b328b89..3e22cd9bcb 100644
--- a/ext/date/lib/parse_posix.c
+++ b/ext/date/lib/parse_posix.c
@@ -489,12 +489,35 @@ timelib_sll timelib_ts_at_start_of_year(timelib_sll year)
 {
 	timelib_sll epoch_leap_years = count_leap_years(1970);
 	timelib_sll current_leap_years = count_leap_years(year);
+	timelib_sll leap_year_diff = current_leap_years - epoch_leap_years; /* not supposed to overflow */
+	timelib_sll val;
 
-	return SECS_PER_DAY * (
-		((year-1970) * DAYS_PER_YEAR)
-		+ current_leap_years
-		- epoch_leap_years
-	);
+	/* cap to valid range */
+	if (year < TIMELIB_SLL_MIN + 1970) {
+		return TIMELIB_SLL_MIN;
+	}
+	val = year - 1970;
+	if (val < TIMELIB_SLL_MIN / DAYS_PER_YEAR) {
+		return TIMELIB_SLL_MIN;
+	}
+	if (val > TIMELIB_SLL_MAX / DAYS_PER_YEAR) {
+		return TIMELIB_SLL_MAX;
+	}
+	val *= DAYS_PER_YEAR;
+	if (val < TIMELIB_SLL_MIN + leap_year_diff) {
+		return TIMELIB_SLL_MIN;
+	}
+	if (val > TIMELIB_SLL_MAX - leap_year_diff) {
+		return TIMELIB_SLL_MAX;
+	}
+	val += leap_year_diff;
+	if (val < TIMELIB_SLL_MIN / SECS_PER_DAY) {
+		return TIMELIB_SLL_MIN;
+	}
+	if (val > TIMELIB_SLL_MAX / SECS_PER_DAY) {
+		return TIMELIB_SLL_MAX;
+	}
+	return val * SECS_PER_DAY;
 }
 
 void timelib_get_transitions_for_year(timelib_tzinfo *tz, timelib_sll year, timelib_posix_transitions *transitions)
diff --git a/ext/date/lib/timelib.h b/ext/date/lib/timelib.h
index a2c976af7e..5bcd94c540 100644
--- a/ext/date/lib/timelib.h
+++ b/ext/date/lib/timelib.h
@@ -131,10 +131,14 @@ typedef uint32_t timelib_ulong;
 typedef uint64_t timelib_ull;
 typedef int64_t timelib_sll;
 # define TIMELIB_LL_CONST(n) n ## i64
+# define TIMELIB_SLL_MIN INT64_MIN
+# define TIMELIB_SLL_MAX INT64_MAX
 #else
 typedef unsigned long long timelib_ull;
 typedef signed long long timelib_sll;
 # define TIMELIB_LL_CONST(n) n ## ll
+# define TIMELIB_SLL_MIN LLONG_MIN
+# define TIMELIB_SLL_MAX LLONG_MAX
 #endif
 
 typedef struct _ttinfo ttinfo;

But then we still get undefined behavior in timelib_get_transitions_for_year() (besides that the code in timelib_ts_at_start_of_year() would almost be unreadable).

If we can assume that timelib_sll is always int64_t, it would get simpler, because we could calculate the allow range for year upfront, and only check that. But still, capping is only a stop-gap measure (certainly better than UB, but not really correct).