Skip to content

sapi: fpm: Add security.exec_basedir per-pool configuration option #1873

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

Closed
wants to merge 1 commit into from
Closed

sapi: fpm: Add security.exec_basedir per-pool configuration option #1873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 17 additions & 0 deletions sapi/fpm/fpm/fpm_conf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ static struct ini_value_parser_s ini_fpm_pool_options[] = {
{ "chdir", &fpm_conf_set_string, WPO(chdir) },
{ "catch_workers_output", &fpm_conf_set_boolean, WPO(catch_workers_output) },
{ "clear_env", &fpm_conf_set_boolean, WPO(clear_env) },
{ "security.exec_basedir", &fpm_conf_set_string, WPO(security_exec_basedir) },
{ "security.limit_extensions", &fpm_conf_set_string, WPO(security_limit_extensions) },
#ifdef HAVE_APPARMOR
{ "apparmor_hat", &fpm_conf_set_string, WPO(apparmor_hat) },
Expand Down Expand Up @@ -654,6 +655,7 @@ int fpm_worker_pool_config_free(struct fpm_worker_pool_config_s *wpc) /* {{{ */
free(wpc->slowlog);
free(wpc->chroot);
free(wpc->chdir);
free(wpc->security_exec_basedir);
free(wpc->security_limit_extensions);
#ifdef HAVE_APPARMOR
free(wpc->apparmor_hat);
Expand Down Expand Up @@ -1016,6 +1018,20 @@ static int fpm_conf_process_all_pools() /* {{{ */
}
}

/* security.exec_basedir */
if (wp->config->security_exec_basedir) {
size_t path_len;

fpm_evaluate_full_path(&wp->config->security_exec_basedir, wp, NULL, 0);

path_len = strlen(wp->config->security_exec_basedir);

if (!path_len || wp->config->security_exec_basedir[path_len - 1] != '/') {
zlog(ZLOG_ERROR, "[pool %s] the security.exec_basedir path '%s' must end with a '/'", wp->config->name, wp->config->security_exec_basedir);
return -1;
}
}

/* security.limit_extensions */
if (!wp->config->security_limit_extensions) {
wp->config->security_limit_extensions = strdup(".php .phar");
Expand Down Expand Up @@ -1642,6 +1658,7 @@ static void fpm_conf_dump() /* {{{ */
zlog(ZLOG_NOTICE, "\tchdir = %s", STR2STR(wp->config->chdir));
zlog(ZLOG_NOTICE, "\tcatch_workers_output = %s", BOOL2STR(wp->config->catch_workers_output));
zlog(ZLOG_NOTICE, "\tclear_env = %s", BOOL2STR(wp->config->clear_env));
zlog(ZLOG_NOTICE, "\tsecurity.exec_basedir = %s", wp->config->security_exec_basedir);
zlog(ZLOG_NOTICE, "\tsecurity.limit_extensions = %s", wp->config->security_limit_extensions);

for (kv = wp->config->env; kv; kv = kv->next) {
Expand Down
1 change: 1 addition & 0 deletions sapi/fpm/fpm/fpm_conf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ struct fpm_worker_pool_config_s {
char *chdir;
int catch_workers_output;
int clear_env;
char *security_exec_basedir;
char *security_limit_extensions;
struct key_value_s *env;
struct key_value_s *php_admin_values;
Expand Down
6 changes: 6 additions & 0 deletions sapi/fpm/fpm/fpm_main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1931,6 +1931,12 @@ consult the installation file that came with this distribution, or visit \n\
goto fastcgi_request_done;
}

if (UNEXPECTED(fpm_php_check_exec_basedir(SG(request_info).path_translated))) {
SG(sapi_headers).http_response_code = 403;
PUTS("Access denied.\n");
goto fastcgi_request_done;
}

if (UNEXPECTED(fpm_php_limit_extensions(SG(request_info).path_translated))) {
SG(sapi_headers).http_response_code = 403;
PUTS("Access denied.\n");
Expand Down
20 changes: 20 additions & 0 deletions sapi/fpm/fpm/fpm_php.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "fpm_worker_pool.h"
#include "zlog.h"

static char *exec_basedir = NULL;
static char **limit_extensions = NULL;

static int fpm_php_zend_ini_alter_master(char *name, int name_length, char *new_value, int new_value_length, int mode, int stage) /* {{{ */
Expand Down Expand Up @@ -221,13 +222,32 @@ int fpm_php_init_child(struct fpm_worker_pool_s *wp) /* {{{ */
return -1;
}

if (wp->config->security_exec_basedir) {
exec_basedir = strdup(wp->config->security_exec_basedir);
}

if (wp->limit_extensions) {
limit_extensions = wp->limit_extensions;
}
return 0;
}
/* }}} */

int fpm_php_check_exec_basedir(char *path) /* {{{ */
{
if (!path || !exec_basedir) {
return 0; /* allowed by default */
}

if (strncmp(path, exec_basedir, strlen(exec_basedir)) == 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Possible read-overflow if strlen(path) < strlen(exec_basedir)

Copy link
Author

Choose a reason for hiding this comment

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

That's not how strncmp works. It will never read off the end of either string.

return 0; /* allow as the exec base dir matches the path */
}

zlog(ZLOG_NOTICE, "Access to the script '%s' has been denied (see security.exec_basedir)", path);
return 1;
}
/* }}} */

int fpm_php_limit_extensions(char *path) /* {{{ */
{
char **p;
Expand Down
1 change: 1 addition & 0 deletions sapi/fpm/fpm/fpm_php.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ size_t fpm_php_content_length(void);
void fpm_php_soft_quit();
int fpm_php_init_main();
int fpm_php_apply_defines_ex(struct key_value_s *kv, int mode);
int fpm_php_check_exec_basedir(char *path);
int fpm_php_limit_extensions(char *path);
char* fpm_php_get_string_from_table(zend_string *table, char *key);

Expand Down
8 changes: 8 additions & 0 deletions sapi/fpm/www.conf.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
; - 'chdir'
; - 'php_values'
; - 'php_admin_values'
; - 'security.exec_basedir'
; When not set, the global prefix (or @php_fpm_prefix@) applies instead.
; Note: This directive can also be relative to the global prefix.
; Default Value: none
Expand Down Expand Up @@ -370,6 +371,13 @@ pm.max_spare_servers = 3
; Default Value: yes
;clear_env = no

; Limits the directory which can be used to execute the request script.
; This can be used to ensure that pools defined for specific users can
; only be used to execute scripts under the control of those users.
; This value must be defined as an absolute path and end with a '/'.
; Default Value: not set
;security.exec_basedir =

; Limits the extensions of the main script FPM will allow to parse. This can
; prevent configuration mistakes on the web server side. You should only limit
; FPM to .php extensions to prevent malicious users to use other extensions to
Expand Down