Potential overflow and sign-extension issues in posix_times() when converting clock_t to zend_long

[DehartShow]

Description

There is a potential security issue when converting clock_t to zend_long that could lead to unpredictable results on different platforms. Platform dependencies of clock_t size: The size of clock_t is not fixed by the C standard, and may differ between systems (32-bit and 64-bit). Sign extension: If clock_t is smaller than zend_long, and clock_t is a signed type, then sign extension may occur when converting to zend_long, leading to incorrect interpretation of the values.

php-src/ext/posix/posix.c

Lines 386 to 390 in 28a083b

	add_assoc_long(return_value, "ticks", ticks); /* clock ticks */
	add_assoc_long(return_value, "utime", t.tms_utime); /* user time */
	add_assoc_long(return_value, "stime", t.tms_stime); /* system time */
	add_assoc_long(return_value, "cutime", t.tms_cutime); /* user time of children */
	add_assoc_long(return_value, "cstime", t.tms_cstime); /* system time of children */

Found using SVACE
Author D. Chizhmak

PHP Version

PHP 8.3+

Operating System

No response

[SakiTakamachi]

As far as I understand, there shouldn’t be any issue in the situation described in the explanation. However, I’m not a native English speaker, so it’s possible that I might be misunderstanding the explanation itself…

[DehartShow]

I highlight a potential issue with the implicit conversion of t.tms_time (type clock_t) to zend_long. Since clock_t's size is platform-dependent, a value that seems valid on one platform might lead to unexpected behavior on another, especially if it's close to the maximum value for a smaller clock_t implementation. Specifically, the sign bit may or may not be correctly extended depending on the platform's architecture and compiler, leading to different interpretations of the value and potentially incorrect results down the line.

[SakiTakamachi]

For example, if clock_t is a signed 32-bit integer type and zend_long is a signed 64-bit integer type, are you concerned that casting from clock_t to zend_long could break or cause issues?

[iluuu1994]

Specifically, the sign bit may or may not be correctly extended depending on the platform's architecture and compiler, leading to different interpretations of the value and potentially incorrect results down the line.

Can you clarify? The C spec does guarantee preserving of integer values when they can be represented by the target type.

C11 §6.3.1.3

When a value with integer type is converted to another integer type other than _Bool, if the value can be represented by the new type, it is unchanged.

If anything, it seems the concern here would be if sizeof(clock_t) > sizeof(zend_long), given that a time with such precision may not be represented by zend_long. However, this doesn't have an easy fix, given this value is stored in zval, which itself may only store zend_long.

[github-actions]

No feedback was provided. The issue is being suspended because we assume that you are no longer experiencing the problem. If this is not the case and you are able to provide the information that was requested earlier, please do so. Thank you.