Skip to content

Fix GH-19320: Prevent FPM UID and GID overflow #22986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
prateekbhujel wants to merge 3 commits into
base: PHP-8.4
Choose a base branch
from
+137 −14
Open

Fix GH-19320: Prevent FPM UID and GID overflow #22986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3f8f9c1
Fix GH-19320: Prevent FPM UID and GID overflow
prateekbhujel Aug 1, 2026
9e2f78b
FPM: Check ID sentinels explicitly
prateekbhujel Aug 11, 2026
d33983b
FPM: Spell out ID type maximums
prateekbhujel Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions NEWS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ PHP NEWS
attribute node that collides by local name with a namespaced
attribute). (David Carlier)

- FPM:
. Fixed bug GH-19320 (FPM UID and GID overflow). (Pratik Bhujel)

- MBString:
. Fixed bug GH-22779 (mb_strrpos() returns the wrong position for a negative
offset in a non-UTF-8 encoding). (Eyüp Can Akman)
Expand Down
85 changes: 73 additions & 12 deletions sapi/fpm/fpm/fpm_unix.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

#include "fpm_config.h"

#include <errno.h>
#include <inttypes.h>
#include <limits.h>
#include <string.h>
#include <sys/time.h>
#include <sys/resource.h>
Expand Down Expand Up @@ -53,6 +56,42 @@ static inline bool fpm_unix_is_id(const char* name)
return strlen(name) == strspn(name, "0123456789");
}

static bool fpm_unix_parse_uid(struct fpm_worker_pool_s *wp, const char *name, uid_t *uid)
{
uintmax_t sentinel = (uintmax_t) ((uid_t) -1);
uintmax_t max = (uid_t) -1 > (uid_t) 0
? (uintmax_t) ((uid_t) -1)
: (UINTMAX_C(1) << (sizeof(uid_t) * CHAR_BIT - 1)) - 1;

@bukka bukka Aug 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should someone re-check but couldn't that shift be in some cases undefined (e.g. on 32bit)?

@prateekbhujel prateekbhujel Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I re-checked this. UINTMAX_C(1) makes the left operand an unsigned uintmax_t, so on a 32-bit system this is a shift by 31 and is defined. uintmax_t is required to be able to represent any unsigned integer type, and this branch is only used when uid_t is signed. So this should be safe here, but I can add a short comment if you think it would make the intent clearer.

@arnaud-lb arnaud-lb Aug 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks right to me. Maybe add a comment to say that max specifically excludes -1 because it's a sentinel value, or check for -1 separately (I was confused by -2 at first).

@prateekbhujel prateekbhujel Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, checking it separately is clearer. I pushed that now: max is the actual type maximum, and (uid_t)-1 / (gid_t)-1 are rejected explicitly as sentinels. The full FPM test suite still passes.


errno = 0;
uintmax_t value = strtoumax(name, NULL, 10);
if (errno == ERANGE || value > max || value == sentinel) {
zlog(ZLOG_ERROR, "[pool %s] user ID '%s' is out of range", wp->config->name, name);
return false;
}

*uid = (uid_t) value;
return true;
}

static bool fpm_unix_parse_gid(struct fpm_worker_pool_s *wp, const char *name, gid_t *gid)
{
uintmax_t sentinel = (uintmax_t) ((gid_t) -1);
uintmax_t max = (gid_t) -1 > (gid_t) 0
? (uintmax_t) ((gid_t) -1)
: (UINTMAX_C(1) << (sizeof(gid_t) * CHAR_BIT - 1)) - 1;

errno = 0;
uintmax_t value = strtoumax(name, NULL, 10);
if (errno == ERANGE || value > max || value == sentinel) {
zlog(ZLOG_ERROR, "[pool %s] group ID '%s' is out of range", wp->config->name, name);
return false;
}

*gid = (gid_t) value;
return true;
}

static struct passwd *fpm_unix_get_passwd(struct fpm_worker_pool_s *wp, const char *name, int flags)
{
struct passwd *pwd = getpwnam(name);
Expand Down Expand Up @@ -93,7 +132,14 @@ static inline bool fpm_unix_check_listen_address(struct fpm_worker_pool_s *wp, c

static inline bool fpm_unix_check_passwd(struct fpm_worker_pool_s *wp, const char *name, int flags)
{
return !name || fpm_unix_is_id(name) || fpm_unix_get_passwd(wp, name, flags);
if (!name || !*name) {
return true;
}
if (fpm_unix_is_id(name)) {
uid_t uid;
return fpm_unix_parse_uid(wp, name, &uid);
}
return fpm_unix_get_passwd(wp, name, flags) != NULL;
}

static struct group *fpm_unix_get_group(struct fpm_worker_pool_s *wp, const char *name, int flags)
Expand All @@ -109,7 +155,14 @@ static struct group *fpm_unix_get_group(struct fpm_worker_pool_s *wp, const char

static inline bool fpm_unix_check_group(struct fpm_worker_pool_s *wp, const char *name, int flags)
{
return !name || fpm_unix_is_id(name) || fpm_unix_get_group(wp, name, flags);
if (!name || !*name) {
return true;
}
if (fpm_unix_is_id(name)) {
gid_t gid;
return fpm_unix_parse_gid(wp, name, &gid);
}
return fpm_unix_get_group(wp, name, flags) != NULL;
}

bool fpm_unix_test_config(struct fpm_worker_pool_s *wp)
Expand All @@ -133,8 +186,8 @@ int fpm_unix_resolve_socket_permissions(struct fpm_worker_pool_s *wp) /* {{{ */
/* uninitialized */
wp->socket_acl = NULL;
#endif
wp->socket_uid = -1;
wp->socket_gid = -1;
wp->socket_uid = (uid_t) -1;
wp->socket_gid = (gid_t) -1;
wp->socket_mode = 0660;

if (!c) {
Expand Down Expand Up @@ -252,7 +305,9 @@ int fpm_unix_resolve_socket_permissions(struct fpm_worker_pool_s *wp) /* {{{ */

if (c->listen_owner && *c->listen_owner) {
if (fpm_unix_is_id(c->listen_owner)) {
wp->socket_uid = strtoul(c->listen_owner, 0, 10);
if (!fpm_unix_parse_uid(wp, c->listen_owner, &wp->socket_uid)) {
return -1;
}
} else {
struct passwd *pwd;

Expand All @@ -268,7 +323,9 @@ int fpm_unix_resolve_socket_permissions(struct fpm_worker_pool_s *wp) /* {{{ */

if (c->listen_group && *c->listen_group) {
if (fpm_unix_is_id(c->listen_group)) {
wp->socket_gid = strtoul(c->listen_group, 0, 10);
if (!fpm_unix_parse_gid(wp, c->listen_group, &wp->socket_gid)) {
return -1;
}
} else {
struct group *grp;

Expand Down Expand Up @@ -325,7 +382,7 @@ int fpm_unix_set_socket_permissions(struct fpm_worker_pool_s *wp, const char *pa
/* When listen.users and listen.groups not configured, continue with standard right */
#endif

if (wp->socket_uid != -1 || wp->socket_gid != -1) {
if (wp->socket_uid != (uid_t) -1 || wp->socket_gid != (gid_t) -1) {
if (0 > chown(path, wp->socket_uid, wp->socket_gid)) {
zlog(ZLOG_SYSERROR, "[pool %s] failed to chown() the socket '%s'", wp->config->name, wp->config->listen_address);
return -1;
Expand Down Expand Up @@ -354,7 +411,9 @@ static int fpm_unix_conf_wp(struct fpm_worker_pool_s *wp) /* {{{ */
if (is_root) {
if (wp->config->user && *wp->config->user) {
if (fpm_unix_is_id(wp->config->user)) {
wp->set_uid = strtoul(wp->config->user, 0, 10);
if (!fpm_unix_parse_uid(wp, wp->config->user, &wp->set_uid)) {
return -1;
}
pwd = getpwuid(wp->set_uid);
if (pwd) {
wp->set_gid = pwd->pw_gid;
Expand All @@ -378,7 +437,9 @@ static int fpm_unix_conf_wp(struct fpm_worker_pool_s *wp) /* {{{ */

if (wp->config->group && *wp->config->group) {
if (fpm_unix_is_id(wp->config->group)) {
wp->set_gid = strtoul(wp->config->group, 0, 10);
if (!fpm_unix_parse_gid(wp, wp->config->group, &wp->set_gid)) {
return -1;
}
} else {
struct group *grp;

Expand Down Expand Up @@ -476,17 +537,17 @@ int fpm_unix_init_child(struct fpm_worker_pool_s *wp) /* {{{ */

if (wp->set_gid) {
if (0 > setgid(wp->set_gid)) {
zlog(ZLOG_SYSERROR, "[pool %s] failed to setgid(%d)", wp->config->name, wp->set_gid);
zlog(ZLOG_SYSERROR, "[pool %s] failed to setgid(%" PRIuMAX ")", wp->config->name, (uintmax_t) wp->set_gid);
return -1;
}
}
if (wp->set_uid) {
if (0 > initgroups(wp->set_user ? wp->set_user : wp->config->user, wp->set_gid)) {
zlog(ZLOG_SYSERROR, "[pool %s] failed to initgroups(%s, %d)", wp->config->name, wp->config->user, wp->set_gid);
zlog(ZLOG_SYSERROR, "[pool %s] failed to initgroups(%s, %" PRIuMAX ")", wp->config->name, wp->config->user, (uintmax_t) wp->set_gid);
return -1;
}
if (0 > setuid(wp->set_uid)) {
zlog(ZLOG_SYSERROR, "[pool %s] failed to setuid(%d)", wp->config->name, wp->set_uid);
zlog(ZLOG_SYSERROR, "[pool %s] failed to setuid(%" PRIuMAX ")", wp->config->name, (uintmax_t) wp->set_uid);
return -1;
}
}
Expand Down
9 changes: 7 additions & 2 deletions sapi/fpm/fpm/fpm_worker_pool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#ifndef FPM_WORKER_POOL_H
#define FPM_WORKER_POOL_H 1

#include <sys/types.h>

#include "fpm_conf.h"
#include "fpm_shm.h"

Expand All @@ -23,9 +25,12 @@ struct fpm_worker_pool_s {
char *user, *home; /* for setting env USER and HOME */
enum fpm_address_domain listen_address_domain;
int listening_socket;
int set_uid, set_gid; /* config uid and gid */
uid_t set_uid;
gid_t set_gid; /* config uid and gid */
char *set_user; /* config user name */
int socket_uid, socket_gid, socket_mode;
uid_t socket_uid;
gid_t socket_gid;
int socket_mode;

/* runtime */
struct fpm_child_s *children;
Expand Down
54 changes: 54 additions & 0 deletions sapi/fpm/tests/gh19320-id-overflow.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
--TEST--
FPM: Reject out-of-range numeric user and group IDs
--SKIPIF--
<?php
include "skipif.inc";
?>
--FILE--
<?php

require_once "tester.inc";

$id = '18446744073709551615';
$settings = [
"user = $id",
"user = 1\ngroup = $id",
"user = 1\nlisten.owner = $id",
"user = 1\nlisten.group = $id",
];

foreach ($settings as $setting) {
$cfg = <<<EOT
[global]
error_log = {{FILE:LOG}}
[unconfined]
listen = {{ADDR:UDS}}
$setting
pm = dynamic
pm.max_children = 5
pm.start_servers = 2
pm.min_spare_servers = 1
pm.max_spare_servers = 3
EOT;

$tester = new FPM\Tester($cfg);
$tester->testConfig();
}

?>
Done
--EXPECT--
ERROR: [pool unconfined] user ID '18446744073709551615' is out of range
ERROR: FPM initialization failed
ERROR: [pool unconfined] group ID '18446744073709551615' is out of range
ERROR: FPM initialization failed
ERROR: [pool unconfined] user ID '18446744073709551615' is out of range
ERROR: FPM initialization failed
ERROR: [pool unconfined] group ID '18446744073709551615' is out of range
ERROR: FPM initialization failed
Done
--CLEAN--
<?php
require_once "tester.inc";
FPM\Tester::clean();
?>
Loading