More ext-random integer overflow

[iluuu1994]

Description

https://github.com/php/php-src/runs/7571517985?check_suite_focus=true

========DIFF========
002+ /home/runner/work/php-src/php-src/ext/random/engine_mt19937.c:175:2: runtime error: signed integer overflow: -9223372036854775808 + -9223372036854775808 cannot be represented in type 'long int'
     Random\Engine\Mt19937: success
     Random\Engine\PcgOneseq128XslRr64: success
     Random\Engine\Xoshiro256StarStar: success
--
========DONE========
FAIL Random: Randomizer: serialize [ext/random/tests/03_randomizer/serialize.phpt] 

Note: This error occurred only in the tests with tracing JIT.

/cc @zeriyoshi

Thanks for taking a look!

PHP Version

master

Operating System

No response

[guilliamxavier]

Curious why it didn't pop up earlier, the "offending" RAND_RANGE_BADSCALING (for legacy mode MT_RAND_PHP) hasn't been changed (only moved) since ~ 5 years old ab834f4, which itself only renamed it from RAND_RANGE, which already was ~ 15 years old (at least) and had had some "fixes" like 8a156d9 or cc28589...

[zeriyoshi]

Probably the same problem: #9191

[TimWolla]

Curious why it didn't pop up earlier, the "offending" RAND_RANGE_BADSCALING (for legacy mode MT_RAND_PHP) hasn't been changed (only moved) since ~ 5 years old

Because mt_rand() was not tested with the full ranges before. Only since the refactoring, were the RNGs methodically exposed with the integer boundaries as inputs. And this issue only really is visible with UBSAN.

[TimWolla]

Standalone reproducer script:

#include<stdint.h>
#include<stdio.h>

typedef int64_t zend_long;
typedef uint64_t zend_ulong;

# define PHP_MT_RAND_MAX ((zend_long) (0x7FFFFFFF))
# define RAND_RANGE_BADSCALING(__n, __min, __max, __tmax) \
	(__n) = (__min) + (zend_long) ((double) ( (double) (__max) - (__min) + 1.0) * ((__n) / ((__tmax) + 1.0)))

zend_ulong bad_scaling(zend_ulong __n, zend_long __min, zend_long __max, zend_long __tmax)
{
	return (__min)
	+
	(zend_long) (
		(double) ( 
			(double) (__max)
			-
			(__min)
			+
			1.0
		)
		*
		(
			(__n)
			/
			(
				(__tmax)
				+
				1.0
			)
		)
	);
}

int
main(void)
{
	zend_long min, max;
	zend_ulong r;

	min = INT64_MIN;
	max = INT64_MAX;
	r = PHP_MT_RAND_MAX - 1;

	// RAND_RANGE_BADSCALING(r, min, max, PHP_MT_RAND_MAX);
	r = bad_scaling(r, min, max, PHP_MT_RAND_MAX);

	printf("%ld\n", (zend_long) r);
}

results in:

$ clang -fsanitize=undefined test.c; ./a.out
test.c:15:2: runtime error: 1.84467e+19 is outside the range of representable values of type 'long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior test.c:15:2 in 
test.c:14:2: runtime error: signed integer overflow: -9223372036854775808 + -9223372036854775808 cannot be represented in type 'long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior test.c:14:2 in 
0

[TimWolla]

Okay, this scaling function is absolutely horrible and we really shouldn't have added the support for MT_RAND_PHP to the Mt19937 engine class. Deprecation that is the first thing I'm going to put up for vote in PHP (8.2)+1.

That said: There's a fix in #9197.

[iluuu1994]

Thanks @TimWolla!