Fix oss-fuzz report triggered by GH-15712 commit.

[devnexen]

It triggered allocation overflow which, even fixed, in turn gives memory leak on 32 bits but the allocator relies on signed integers so instead of changing j type we exit if an overflow during the buffer increase is going to happen.

[cmb69]

Maybe we should constrain precision (and related) in the first place (and give users some hint):

php-src/main/main.c

Lines 289 to 298 in c7397f5

	static PHP_INI_MH(OnSetPrecision)
	{
	zend_long i = ZEND_ATOL(ZSTR_VAL(new_value));
	if (i >= -1) {
	EG(precision) = i;
	return SUCCESS;
	} else {
	return FAILURE;
	}
	}

I mean, even precision=100 makes no sense; you just get some made-up garbage (https://3v4l.org/eQinL).

[devnexen]

for sure, but I was thinking it s behavior change, precision never was limited and I am targeting a stable branch. Thought it was more appropriate for master.

[cmb69]

Thought it was more appropriate for master.

Oh, absolutely. :)

[cmb69]

If the loop is terminated because j > (INT_MAX >> 1), isn't there the risk of a buffer overflow in the following? I think in this case we would need to bail out (exception or even a fatal error).

And if we bail out, we could also check whether i exceeds the threshold before even entering the loop.

And from there we can even move the check up the stack; likely zend_dtoa() should not accept very large ndigits.

[cmb69]

I think this line should be

Suggested change

	if (i > (INT_MAX - rem))
	if (i > ((INT_MAX >> 2) + rem))

Then the other overflow check should be superfluous.

[cmb69]

Oh, and I think reverting the changes from #15715 could also be appropriate, since j should never exceed INT_MAX (actually, it must always be less than 0x40000000).

[cmb69]

Thank you! That should fix it.

[cmb69]

Oh, I've overlooked that the test leaks now. We could mark it as --XFAIL--, or silently clamp the value to its allowed maximum (I don't think anybody will notice :).