Skip to content

Commit 7dc17df

Browse files
bukkabukka
committed
Fix GH-17645: FPM with httpd ProxyPass does not decode script path
This always decodes SCRIPT_FILENAME when Apache ProxyPass or ProxyPassMatch is used. It also introduces a new INI option fastcgi.script_path_encoded that allows using previous behavior of not decoding as there is a chance that some users could use encoded file paths in FS.
1 parent 77b6a79 commit 7dc17df

6 files changed

+150
-13
lines changed

‎php.ini-development

+5
Original file line numberDiff line numberDiff line change
@@ -817,6 +817,11 @@ enable_dl = Off
817817
; https://php.net/fastcgi.impersonate
818818
;fastcgi.impersonate = 1
819819

820+
; Prevent decoding of SCRIPT_FILENAME if Apache ProxyPass or ProxyPassMatch is
821+
; used. This should not be used unless the script file paths are saved already
822+
; encoded in the file system.
823+
;fastcgi.script_path_encoded = 1
824+
820825
; Disable logging through FastCGI connection. PHP's default behavior is to enable
821826
; this feature.
822827
;fastcgi.logging = 0

‎php.ini-production

+5
Original file line numberDiff line numberDiff line change
@@ -819,6 +819,11 @@ enable_dl = Off
819819
; https://php.net/fastcgi.impersonate
820820
;fastcgi.impersonate = 1
821821

822+
; Prevent decoding of SCRIPT_FILENAME if Apache ProxyPass or ProxyPassMatch is
823+
; used. This should not be used unless the script file paths are saved already
824+
; encoded in the file system.
825+
;fastcgi.script_path_encoded = 1
826+
822827
; Disable logging through FastCGI connection. PHP's default behavior is to enable
823828
; this feature.
824829
;fastcgi.logging = 0

‎sapi/fpm/fpm/fpm_main.c

+15-8
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ typedef struct _php_cgi_globals_struct {
144144
bool nph;
145145
bool fix_pathinfo;
146146
bool discard_path;
147+
bool fcgi_script_path_encoded;
147148
bool fcgi_logging;
148149
bool fcgi_logging_request_started;
149150
HashTable user_config_cache;
@@ -1066,6 +1067,10 @@ static void init_request_info(void)
10661067
}
10671068
}
10681069

1070+
if (apache_was_here && !CGIG(fcgi_script_path_encoded)) {
1071+
php_raw_url_decode(env_script_filename, strlen(env_script_filename));
1072+
}
1073+
10691074
if (CGIG(fix_pathinfo)) {
10701075
struct stat st;
10711076
char *real_path = NULL;
@@ -1174,7 +1179,7 @@ static void init_request_info(void)
11741179
* it is probably also in SCRIPT_NAME and need to be removed
11751180
*/
11761181
size_t decoded_path_info_len = 0;
1177-
if (strchr(path_info, '%')) {
1182+
if (CGIG(fcgi_script_path_encoded) && strchr(path_info, '%')) {
11781183
decoded_path_info = estrdup(path_info);
11791184
decoded_path_info_len = php_raw_url_decode(decoded_path_info, strlen(path_info));
11801185
}
@@ -1421,13 +1426,14 @@ static void fastcgi_ini_parser(zval *arg1, zval *arg2, zval *arg3, int callback_
14211426
/* }}} */
14221427

14231428
PHP_INI_BEGIN()
1424-
STD_PHP_INI_BOOLEAN("cgi.rfc2616_headers", "0", PHP_INI_ALL, OnUpdateBool, rfc2616_headers, php_cgi_globals_struct, php_cgi_globals)
1425-
STD_PHP_INI_BOOLEAN("cgi.nph", "0", PHP_INI_ALL, OnUpdateBool, nph, php_cgi_globals_struct, php_cgi_globals)
1426-
STD_PHP_INI_BOOLEAN("cgi.fix_pathinfo", "1", PHP_INI_SYSTEM, OnUpdateBool, fix_pathinfo, php_cgi_globals_struct, php_cgi_globals)
1427-
STD_PHP_INI_BOOLEAN("cgi.discard_path", "0", PHP_INI_SYSTEM, OnUpdateBool, discard_path, php_cgi_globals_struct, php_cgi_globals)
1428-
STD_PHP_INI_BOOLEAN("fastcgi.logging", "1", PHP_INI_SYSTEM, OnUpdateBool, fcgi_logging, php_cgi_globals_struct, php_cgi_globals)
1429-
STD_PHP_INI_ENTRY("fastcgi.error_header", NULL, PHP_INI_SYSTEM, OnUpdateString, error_header, php_cgi_globals_struct, php_cgi_globals)
1430-
STD_PHP_INI_ENTRY("fpm.config", NULL, PHP_INI_SYSTEM, OnUpdateString, fpm_config, php_cgi_globals_struct, php_cgi_globals)
1429+
STD_PHP_INI_BOOLEAN("cgi.rfc2616_headers", "0", PHP_INI_ALL, OnUpdateBool, rfc2616_headers, php_cgi_globals_struct, php_cgi_globals)
1430+
STD_PHP_INI_BOOLEAN("cgi.nph", "0", PHP_INI_ALL, OnUpdateBool, nph, php_cgi_globals_struct, php_cgi_globals)
1431+
STD_PHP_INI_BOOLEAN("cgi.fix_pathinfo", "1", PHP_INI_SYSTEM, OnUpdateBool, fix_pathinfo, php_cgi_globals_struct, php_cgi_globals)
1432+
STD_PHP_INI_BOOLEAN("cgi.discard_path", "0", PHP_INI_SYSTEM, OnUpdateBool, discard_path, php_cgi_globals_struct, php_cgi_globals)
1433+
STD_PHP_INI_BOOLEAN("fastcgi.script_path_encoded", "0", PHP_INI_SYSTEM, OnUpdateBool, fcgi_script_path_encoded, php_cgi_globals_struct, php_cgi_globals)
1434+
STD_PHP_INI_BOOLEAN("fastcgi.logging", "1", PHP_INI_SYSTEM, OnUpdateBool, fcgi_logging, php_cgi_globals_struct, php_cgi_globals)
1435+
STD_PHP_INI_ENTRY("fastcgi.error_header", NULL, PHP_INI_SYSTEM, OnUpdateString, error_header, php_cgi_globals_struct, php_cgi_globals)
1436+
STD_PHP_INI_ENTRY("fpm.config", NULL, PHP_INI_SYSTEM, OnUpdateString, fpm_config, php_cgi_globals_struct, php_cgi_globals)
14311437
PHP_INI_END()
14321438

14331439
/* {{{ php_cgi_globals_ctor */
@@ -1437,6 +1443,7 @@ static void php_cgi_globals_ctor(php_cgi_globals_struct *php_cgi_globals)
14371443
php_cgi_globals->nph = 0;
14381444
php_cgi_globals->fix_pathinfo = 1;
14391445
php_cgi_globals->discard_path = 0;
1446+
php_cgi_globals->fcgi_script_path_encoded = 0;
14401447
php_cgi_globals->fcgi_logging = 1;
14411448
php_cgi_globals->fcgi_logging_request_started = false;
14421449
zend_hash_init(&php_cgi_globals->user_config_cache, 0, NULL, user_config_cache_entry_dtor, 1);

‎sapi/fpm/tests/fcgi-env-pif-apache-pp-sfp-decoding.phpt

+54
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
--TEST--
2+
FPM: FastCGI env var path info fix for Apache ProxyPass SCRIPT_FILEPATH decoding (GH-17645)
3+
--SKIPIF--
4+
<?php include "skipif.inc"; ?>
5+
--FILE--
6+
<?php
7+
8+
require_once "tester.inc";
9+
10+
$cfg = <<<EOT
11+
[global]
12+
error_log = {{FILE:LOG}}
13+
[unconfined]
14+
listen = {{ADDR}}
15+
pm = dynamic
16+
pm.max_children = 5
17+
pm.start_servers = 1
18+
pm.min_spare_servers = 1
19+
pm.max_spare_servers = 3
20+
php_admin_value[cgi.fix_pathinfo] = yes
21+
EOT;
22+
23+
$code = <<<EOT
24+
<?php
25+
echo \$_SERVER["SCRIPT_NAME"] . "\n";
26+
echo \$_SERVER["ORIG_SCRIPT_NAME"] . "\n";
27+
echo \$_SERVER["SCRIPT_FILENAME"] . "\n";
28+
echo \$_SERVER["PATH_INFO"] . "\n";
29+
echo \$_SERVER["PHP_SELF"];
30+
EOT;
31+
32+
$tester = new FPM\Tester($cfg, $code);
33+
[$sourceFilePath, $scriptName] = $tester->createSourceFileAndScriptName('+');
34+
$tester->start();
35+
$tester->expectLogStartNotices();
36+
$tester
37+
->request(
38+
uri: $scriptName . '/1%202',
39+
scriptFilename: "proxy:fcgi://" . $tester->getAddr() . str_replace('+', '%2B', $sourceFilePath) . '/1%202',
40+
scriptName: $scriptName . '/1 2'
41+
)
42+
->expectBody([$scriptName, $scriptName . '/1 2', $sourceFilePath, '/1 2', $scriptName . '/1 2']);
43+
$tester->terminate();
44+
$tester->close();
45+
46+
?>
47+
Done
48+
--EXPECT--
49+
Done
50+
--CLEAN--
51+
<?php
52+
require_once "tester.inc";
53+
FPM\Tester::clean();
54+
?>

‎sapi/fpm/tests/fcgi-env-pif-apache-pp-sfp-encoded.phpt

+55
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
--TEST--
2+
FPM: FastCGI env var path info fix for Apache ProxyPass SCRIPT_FILEPATH keep encoded (GH-17645)
3+
--SKIPIF--
4+
<?php include "skipif.inc"; ?>
5+
--FILE--
6+
<?php
7+
8+
require_once "tester.inc";
9+
10+
$cfg = <<<EOT
11+
[global]
12+
error_log = {{FILE:LOG}}
13+
[unconfined]
14+
listen = {{ADDR}}
15+
pm = dynamic
16+
pm.max_children = 5
17+
pm.start_servers = 1
18+
pm.min_spare_servers = 1
19+
pm.max_spare_servers = 3
20+
php_admin_value[cgi.fix_pathinfo] = yes
21+
php_admin_value[fastcgi.script_path_encoded] = yes
22+
EOT;
23+
24+
$code = <<<EOT
25+
<?php
26+
echo \$_SERVER["SCRIPT_NAME"] . "\n";
27+
echo \$_SERVER["ORIG_SCRIPT_NAME"] . "\n";
28+
echo \$_SERVER["SCRIPT_FILENAME"] . "\n";
29+
echo \$_SERVER["PATH_INFO"] . "\n";
30+
echo \$_SERVER["PHP_SELF"];
31+
EOT;
32+
33+
$tester = new FPM\Tester($cfg, $code);
34+
[$sourceFilePath, $scriptName] = $tester->createSourceFileAndScriptName('%2B');
35+
$tester->start();
36+
$tester->expectLogStartNotices();
37+
$tester
38+
->request(
39+
uri: $scriptName . '/1%202',
40+
scriptFilename: "proxy:fcgi://" . $tester->getAddr() . $sourceFilePath . '/1%202',
41+
scriptName: $scriptName . '/1 2'
42+
)
43+
->expectBody([$scriptName, $scriptName . '/1 2', $sourceFilePath, '/1 2', $scriptName . '/1 2']);
44+
$tester->terminate();
45+
$tester->close();
46+
47+
?>
48+
Done
49+
--EXPECT--
50+
Done
51+
--CLEAN--
52+
<?php
53+
require_once "tester.inc";
54+
FPM\Tester::clean();
55+
?>

‎sapi/fpm/tests/tester.inc

+16-5
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ class Tester
4747
self::FILE_EXT_LOG_ERR,
4848
self::FILE_EXT_LOG_SLOW,
4949
self::FILE_EXT_PID,
50-
'src.php',
50+
'*src.php',
5151
'ini',
5252
'skip.ini',
5353
'*.sock',
@@ -107,6 +107,11 @@ class Tester
107107
*/
108108
private string $fileName;
109109

110+
/**
111+
* @var string
112+
*/
113+
private ?string $sourceFile = null;
114+
110115
/**
111116
* @var resource
112117
*/
@@ -1483,21 +1488,27 @@ class Tester
14831488
/**
14841489
* Create a source code file.
14851490
*
1491+
* @param string $nameSuffix
14861492
* @return string
14871493
*/
1488-
public function makeSourceFile(): string
1494+
public function makeSourceFile(string $nameSuffix = ''): string
14891495
{
1490-
return $this->makeFile('src.php', $this->code, overwrite: false);
1496+
if (is_null($this->sourceFile)) {
1497+
$this->sourceFile = $this->makeFile($nameSuffix . 'src.php', $this->code, overwrite: false);
1498+
}
1499+
1500+
return $this->sourceFile;
14911501
}
14921502

14931503
/**
14941504
* Create a source file and script name.
14951505
*
1506+
* @param string $nameSuffix
14961507
* @return string[]
14971508
*/
1498-
public function createSourceFileAndScriptName(): array
1509+
public function createSourceFileAndScriptName(string $nameSuffix = ''): array
14991510
{
1500-
$sourceFile = $this->makeFile('src.php', $this->code, overwrite: false);
1511+
$sourceFile = $this->makeSourceFile($nameSuffix);
15011512

15021513
return [$sourceFile, '/' . basename($sourceFile)];
15031514
}

0 commit comments

Comments
 (0)