ext/posix: posix_kill() process_id range check.

pid_t is, for the most part, represented by a signed int, by overflowing
it, we end up being in the -1 case which affect all accessible processes.

close GH-18944

Author: devnexen

NEWS

@@ -28,6 +28,10 @@ PHP                                                                        NEWS
   . Add $digest_algo parameter to openssl_public_encrypt() and
     openssl_private_decrypt() functions. (Jakub Zelenka)
 
+- POSIX:
+  . posix_kill and posix_setpgid throws a ValueError on invalid process_id.
+    (David Carlier)
+
 - Reflection:
   . Fixed bug GH-19187 (ReflectionNamedType::getName() prints nullable type when
     retrieved from ReflectionProperty::getSettableType()). (ilutov)

UPGRADING

@@ -381,6 +381,10 @@ PHP 8.5 UPGRADE NOTES
     an invalid file descriptor.
   . posix_fpathconf checks invalid file descriptors and sets
     last_error to EBADF and raises an E_WARNING message.
+  . posix_kill throws a ValueError when the process_id argument is lower
+    or greater than what supports the platform (signed integer or long
+    range), posix_setpgid throws a ValueError when the process_id is
+    lower than zero or greater than what supports the platform.
 
 - Reflection:
   . The output of ReflectionClass::toString() for enums has changed to

ext/posix/posix.c

@@ -45,6 +45,14 @@
 # include <sys/sysmacros.h>
 #endif
 
+#if (defined(__sun) && !defined(_LP64)) || defined(_AIX)
+#define POSIX_PID_MIN LONG_MIN
+#define POSIX_PID_MAX LONG_MAX
+#else
+#define POSIX_PID_MIN INT_MIN
+#define POSIX_PID_MAX INT_MAX
+#endif
+
 #include "posix_arginfo.h"
 
 ZEND_DECLARE_MODULE_GLOBALS(posix)
@@ -118,6 +126,12 @@ ZEND_GET_MODULE(posix)
 	}	\
 	RETURN_TRUE;
 
+#define PHP_POSIX_CHECK_PID(pid, lower, upper)										\
+	if (pid < lower || pid > upper) {										\
+		zend_argument_value_error(1, "must be between " ZEND_LONG_FMT " and " ZEND_LONG_FMT, lower, upper);	\
+		RETURN_THROWS();											\
+	}
+
 /* {{{ Send a signal to a process (POSIX.1, 3.3.2) */
 
 PHP_FUNCTION(posix_kill)
@@ -129,6 +143,8 @@ PHP_FUNCTION(posix_kill)
 		Z_PARAM_LONG(sig)
 	ZEND_PARSE_PARAMETERS_END();
 
+	PHP_POSIX_CHECK_PID(pid, POSIX_PID_MIN, POSIX_PID_MAX)
+
 	if (kill(pid, sig) < 0) {
 		POSIX_G(last_error) = errno;
 		RETURN_FALSE;
@@ -291,6 +307,8 @@ PHP_FUNCTION(posix_setpgid)
 		Z_PARAM_LONG(pgid)
 	ZEND_PARSE_PARAMETERS_END();
 
+	PHP_POSIX_CHECK_PID(pid, 0, POSIX_PID_MAX)
+
 	if (setpgid(pid, pgid) < 0) {
 		POSIX_G(last_error) = errno;
 		RETURN_FALSE;

ext/posix/tests/posix_kill_pidoverflow.phpt

@@ -0,0 +1,24 @@
+--TEST--
+posix_kill() with large pid
+--EXTENSIONS--
+posix
+--SKIPIF--
+<?php if (PHP_INT_SIZE != 8) die("skip this test is for 64bit platform only"); ?>
+--FILE--
+<?php
+// with pid overflow, it ends up being -1 which means all permissible processes are affected
+try {
+	posix_kill(PHP_INT_MAX, SIGTERM);
+} catch (\ValueError $e) {
+	echo $e->getMessage(), PHP_EOL;
+}
+
+try {
+	posix_kill(PHP_INT_MIN, SIGTERM);
+} catch (\ValueError $e) {
+	echo $e->getMessage(), PHP_EOL;
+}
+?>
+--EXPECTF--
+posix_kill(): Argument #1 ($process_id) must be between %i and %d
+posix_kill(): Argument #1 ($process_id) must be between %i and %d

ext/posix/tests/posix_setpgid_error.phpt

@@ -0,0 +1,22 @@
+--TEST--
+posix_setpgid() with wrong pid values
+--EXTENSIONS--
+posix
+--SKIPIF--
+<?php if (PHP_INT_SIZE != 8) die("skip this test is for 64bit platform only"); ?>
+--FILE--
+<?php
+try {
+	posix_setpgid(PHP_INT_MAX, 1);
+} catch (\ValueError $e) {
+	echo $e->getMessage(), PHP_EOL;
+}
+try {
+	posix_setpgid(-2, 1);
+} catch (\ValueError $e) {
+	echo $e->getMessage(), PHP_EOL;
+}
+?>
+--EXPECTF--
+posix_setpgid(): Argument #1 ($process_id) must be between 0 and %d
+posix_setpgid(): Argument #1 ($process_id) must be between 0 and %d