Skip to content

Commit c540a26

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 46213f8 commit c540a26

File tree

4 files changed

+63
-1
lines changed

4 files changed

+63
-1
lines changed

ext/posix/posix.c

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

48+
#if (defined(__sun) && !defined(_LP64)) || defined(_AIX)
49+
#define POSIX_PID_MAX LONG_MAX
50+
#else
51+
#define POSIX_PID_MAX INT_MAX
52+
#endif
53+
4854
#include "posix_arginfo.h"
4955

5056
ZEND_DECLARE_MODULE_GLOBALS(posix)
@@ -118,6 +124,12 @@ ZEND_GET_MODULE(posix)
118124
} \
119125
RETURN_TRUE;
120126

127+
#define PHP_POSIX_CHECK_PID(pid) \
128+
if (pid < -1 || pid > POSIX_PID_MAX) { \
129+
zend_argument_value_error(1, "must be between -1 and " ZEND_LONG_FMT, POSIX_PID_MAX); \
130+
RETURN_THROWS(); \
131+
}
132+
121133
/* {{{ Send a signal to a process (POSIX.1, 3.3.2) */
122134

123135
PHP_FUNCTION(posix_kill)
@@ -129,6 +141,8 @@ PHP_FUNCTION(posix_kill)
129141
Z_PARAM_LONG(sig)
130142
ZEND_PARSE_PARAMETERS_END();
131143

144+
PHP_POSIX_CHECK_PID(pid)
145+
132146
if (kill(pid, sig) < 0) {
133147
POSIX_G(last_error) = errno;
134148
RETURN_FALSE;
@@ -291,6 +305,8 @@ PHP_FUNCTION(posix_setpgid)
291305
Z_PARAM_LONG(pgid)
292306
ZEND_PARSE_PARAMETERS_END();
293307

308+
PHP_POSIX_CHECK_PID(pid)
309+
294310
if (setpgid(pid, pgid) < 0) {
295311
POSIX_G(last_error) = errno;
296312
RETURN_FALSE;

ext/posix/tests/posix_kill_error.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ $sig = 999;
1313
var_dump( posix_kill($pid, 999) );
1414

1515
echo "\n-- Testing posix_kill() function with negative pid --\n";
16-
$pid = -999;
16+
$pid = -1;
1717
$sig = 9;
1818
var_dump( posix_kill($pid, 999) );
1919

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 -1 and %d
24+
posix_kill(): Argument #1 ($process_id) must be between -1 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 -1 and %d
22+
posix_setpgid(): Argument #1 ($process_id) must be between -1 and %d

0 commit comments

Comments
 (0)