Skip to content

Commit bc69cef

Browse files
committed
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.
1 parent 1089896 commit bc69cef

File tree

3 files changed

+64
-0
lines changed

3 files changed

+64
-0
lines changed

ext/posix/posix.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,14 @@
4545
# include <sys/sysmacros.h>
4646
#endif
4747

48+
#if (defined(__sun) && !defined(_LP64)) || defined(_AIX)
49+
#define POSIX_PID_MIN LONG_MIN
50+
#define POSIX_PID_MAX LONG_MAX
51+
#else
52+
#define POSIX_PID_MIN INT_MIN
53+
#define POSIX_PID_MAX INT_MAX
54+
#endif
55+
4856
#include "posix_arginfo.h"
4957

5058
ZEND_DECLARE_MODULE_GLOBALS(posix)
@@ -118,6 +126,12 @@ ZEND_GET_MODULE(posix)
118126
} \
119127
RETURN_TRUE;
120128

129+
#define PHP_POSIX_CHECK_PID(pid, lower, upper) \
130+
if (pid < lower || pid > upper) { \
131+
zend_argument_value_error(1, "must be between " ZEND_LONG_FMT " and " ZEND_LONG_FMT, lower, upper); \
132+
RETURN_THROWS(); \
133+
}
134+
121135
/* {{{ Send a signal to a process (POSIX.1, 3.3.2) */
122136

123137
PHP_FUNCTION(posix_kill)
@@ -129,6 +143,8 @@ PHP_FUNCTION(posix_kill)
129143
Z_PARAM_LONG(sig)
130144
ZEND_PARSE_PARAMETERS_END();
131145

146+
PHP_POSIX_CHECK_PID(pid, POSIX_PID_MIN, POSIX_PID_MAX)
147+
132148
if (kill(pid, sig) < 0) {
133149
POSIX_G(last_error) = errno;
134150
RETURN_FALSE;
@@ -291,6 +307,8 @@ PHP_FUNCTION(posix_setpgid)
291307
Z_PARAM_LONG(pgid)
292308
ZEND_PARSE_PARAMETERS_END();
293309

310+
PHP_POSIX_CHECK_PID(pid, 0, POSIX_PID_MAX)
311+
294312
if (setpgid(pid, pgid) < 0) {
295313
POSIX_G(last_error) = errno;
296314
RETURN_FALSE;
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
--TEST--
2+
posix_kill() with large pid
3+
--EXTENSIONS--
4+
posix
5+
--SKIPIF--
6+
<?php if (PHP_INT_SIZE != 8) die("skip this test is for 64bit platform only"); ?>
7+
--FILE--
8+
<?php
9+
// with pid overflow, it ends up being -1 which means all permissible processes are affected
10+
try {
11+
posix_kill(PHP_INT_MAX, SIGTERM);
12+
} catch (\ValueError $e) {
13+
echo $e->getMessage(), PHP_EOL;
14+
}
15+
16+
try {
17+
posix_kill(PHP_INT_MIN, SIGTERM);
18+
} catch (\ValueError $e) {
19+
echo $e->getMessage(), PHP_EOL;
20+
}
21+
?>
22+
--EXPECTF--
23+
posix_kill(): Argument #1 ($process_id) must be between %i and %d
24+
posix_kill(): Argument #1 ($process_id) must be between %i and %d
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
--TEST--
2+
posix_setpgid() with wrong pid values
3+
--EXTENSIONS--
4+
posix
5+
--SKIPIF--
6+
<?php if (PHP_INT_SIZE != 8) die("skip this test is for 64bit platform only"); ?>
7+
--FILE--
8+
<?php
9+
try {
10+
posix_setpgid(PHP_INT_MAX, 1);
11+
} catch (\ValueError $e) {
12+
echo $e->getMessage(), PHP_EOL;
13+
}
14+
try {
15+
posix_setpgid(-2, 1);
16+
} catch (\ValueError $e) {
17+
echo $e->getMessage(), PHP_EOL;
18+
}
19+
?>
20+
--EXPECTF--
21+
posix_setpgid(): Argument #1 ($process_id) must be between 0 and %d
22+
posix_setpgid(): Argument #1 ($process_id) must be between 0 and %d

0 commit comments

Comments
 (0)