Skip to content

Commit 278f80a

Browse files
DanielEScherzerDanielEScherzer
committed
Tests, fixed logic
1 parent 5894d7f commit 278f80a

14 files changed

Lines changed: 130 additions & 65 deletions

‎sapi/cgi/cgi_main.c‎

Lines changed: 22 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1687,59 +1687,41 @@ static void add_response_header(sapi_header_struct *h, zval *return_value) /* {{
16871687
}
16881688
/* }}} */
16891689

1690-
zend_result cgi_limit_extensions(char *path) {
1690+
bool cgi_extension_is_allowed(char *path) {
16911691
const char *allowed_extensions = CGIG(security_limit_extensions);
16921692
if (!path || !allowed_extensions || strlen(allowed_extensions) == 0) {
16931693
// No path, or no filter configured
1694-
return SUCCESS;
1694+
return true;
16951695
}
16961696

16971697
const char *dot = strrchr(path, '.');
16981698
if (!dot) {
16991699
// No file extension
1700-
return FAILURE;
1700+
return false;
17011701
}
17021702

1703-
#ifndef PHP_WIN32
1704-
const char dir_separator = '\\';
1705-
#else
1706-
const char dir_separator = '/';
1707-
#endif
1708-
1709-
if (strchr(dot, dir_separator) != NULL) {
1710-
/* Dot is followed by a directory separator at some point, so this is
1711-
* a dot in a directory name, rather than the actual file extension;
1712-
* strrchr() above means that this was the last dot, so the actual file
1713-
* has no extension. */
1714-
return FAILURE;
1715-
}
1716-
1717-
/* `dot` is now the pointer to the start of a null-terminated string for
1718-
* the file extension, *including* the dot, e.g. `.php`. We need that string
1719-
* to be present in the allowed extensions, and
1720-
* - preceded by either a space or tab, or be at the start of the string,
1721-
* so that `.template.php` can be allowed without allowing all `.php`
1722-
* files
1723-
* - followed by either a space or tab, or be at the end of the string, for
1724-
* the same reason, `.php.trusted` should not allow all of `.php`
1725-
*/
1726-
char *next = strstr(allowed_extensions, dot);
1727-
const size_t extension_len = strlen(dot);
1703+
/* We want to be able to allow things like `.test.php` (but not all of
1704+
* `.php` or `.test`. For each extension in the list, check if the path
1705+
* given ends with exactly that. Allowing `.php` allows `.test.php`,
1706+
* `.foo.php`, and everything else because dots are valid file name
1707+
* characters. */
1708+
const char *next = allowed_extensions;
1709+
const size_t path_len = strlen(path);
17281710
while (next && *next) {
1729-
/* We have a pointer to within the allowed extensions, which is followed
1730-
* by the extension; adding extension_len will bring us to either the
1731-
* character after the extension, or the null terminating byte. */
1732-
char *after = next + extension_len;
1733-
if (
1734-
(next == allowed_extensions || (*(next - 1) == ' ') || (*(next - 1) == '\t'))
1735-
&& (*after == '\0' || *after == ' ' || *after == '\t')
1736-
) {
1737-
return SUCCESS;
1711+
size_t extension_len = strcspn(next, " \t");
1712+
1713+
if (extension_len <= path_len) {
1714+
if (memcmp(path + path_len - extension_len, next, extension_len) == 0) {
1715+
return true;
1716+
}
1717+
}
1718+
next += extension_len;
1719+
if (*next) {
1720+
next += strspn(next, " \t");
17381721
}
1739-
next = strstr(next + 1, dot);
17401722
}
17411723

1742-
return FAILURE;
1724+
return false;
17431725
}
17441726

17451727
PHP_FUNCTION(apache_response_headers) /* {{{ */
@@ -2533,7 +2515,7 @@ consult the installation file that came with this distribution, or visit \n\
25332515
2. we are running as cgi or fastcgi
25342516
*/
25352517
if (cgi || fastcgi || SG(request_info).path_translated) {
2536-
bool limited_extension = (cgi_limit_extensions(SG(request_info).path_translated) == FAILURE);
2518+
bool limited_extension = !cgi_extension_is_allowed(SG(request_info).path_translated);
25372519
if (limited_extension || php_fopen_primary_script(&file_handle) == FAILURE) {
25382520
zend_try {
25392521
if (limited_extension || errno == EACCES) {

‎sapi/cgi/tests/003.phpt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@ class test { /* {{{ */
3838
file_put_contents($filename, $code);
3939

4040
var_dump(shell_exec(<<<SHELL
41-
$php -n -w "$filename"
41+
$php -n -d cgi.security_limit_extensions=".test.php" -w "$filename"
4242
SHELL));
4343
var_dump(shell_exec(<<<SHELL
44-
$php -n -w "wrong"
44+
$php -n -d cgi.security_limit_extensions="" -w "wrong"
4545
SHELL));
4646
var_dump(shell_exec(<<<SHELL
4747
echo "<?php /* comment */ class test {\n // comment \n function foo() {} } ?>" | $php -n -w

‎sapi/cgi/tests/004.phpt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,15 @@ file_put_contents($filename, $code);
2828

2929
if (defined("PHP_WINDOWS_VERSION_MAJOR")) {
3030
var_dump(shell_exec(<<<SHELL
31-
$php -n -f "$filename"
31+
$php -n -d cgi.security_limit_extensions='.test.php' -f "$filename"
3232
SHELL));
3333
} else {
3434
var_dump(shell_exec(<<<SHELL
35-
$php -n -f "$filename" 2>/dev/null
35+
$php -n -d cgi.security_limit_extensions='.test.php' -f "$filename" 2>/dev/null
3636
SHELL));
3737
}
3838
var_dump(shell_exec(<<<SHELL
39-
$php -n -f "wrong"
39+
$php -n -d cgi.security_limit_extensions='' -f "wrong"
4040
SHELL));
4141

4242
@unlink($filename);

‎sapi/cgi/tests/005.phpt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@ $php = get_cgi_path();
1111
reset_env_vars();
1212

1313
var_dump(shell_exec(<<<SHELL
14-
$php -n -a -f "wrong"
14+
$php -n -d cgi.security_limit_extensions="" -a -f "wrong"
1515
SHELL));
1616
var_dump(shell_exec(<<<SHELL
17-
$php -n -f "wrong" -a
17+
$php -n -d cgi.security_limit_extensions="" -f "wrong" -a
1818
SHELL));
1919

2020
echo "Done\n";

‎sapi/cgi/tests/006.phpt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ echo test::$var;
3030
file_put_contents($filename, $code);
3131

3232
var_dump(shell_exec(<<<SHELL
33-
"$php" -n -l "$filename"
33+
"$php" -n -d cgi.security_limit_extensions=".test.php" -l "$filename"
3434
SHELL));
3535
var_dump(shell_exec(<<<SHELL
36-
"$php" -n -l some.unknown
36+
"$php" -n -d cgi.security_limit_extensions='.unknown' -l some.unknown
3737
SHELL));
3838

3939
$code = '
@@ -50,11 +50,11 @@ file_put_contents($filename, $code);
5050

5151
if (defined("PHP_WINDOWS_VERSION_MAJOR")) {
5252
var_dump(shell_exec(<<<SHELL
53-
"$php" -n -l "$filename"
53+
"$php" -n -d cgi.security_allowed_extensions=".test.php" -l "$filename"
5454
SHELL));
5555
} else {
5656
var_dump(shell_exec(<<<SHELL
57-
"$php" -n -l "$filename" 2>/dev/null
57+
"$php" -n -d cgi.security_allowed_extensions=".test.php" -l "$filename" 2>/dev/null
5858
SHELL));
5959
}
6060

‎sapi/cgi/tests/008.phpt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@ $o = new test;
3131
file_put_contents($filename, $code);
3232

3333
var_dump(shell_exec(<<<SHELL
34-
"$php" -n -s "$filename"
34+
"$php" -n -d cgi.security_limit_extensions='.test.php' -s "$filename"
3535
SHELL));
3636
var_dump(shell_exec(<<<SHELL
37-
"$php" -n -s "unknown"
37+
"$php" -n -d cgi.security_limit_extensions='' -s "unknown"
3838
SHELL));
3939

4040
@unlink($filename);

‎sapi/cgi/tests/009.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ putenv("TRANSLATED_PATH=".$f."/x");
1616
putenv("SCRIPT_FILENAME=".$f."/x");
1717
file_put_contents($f, '<?php var_dump($_SERVER["TRANSLATED_PATH"]); ?>');
1818

19-
echo shell_exec("$php -n $f");
19+
echo shell_exec("$php -n -d cgi.security_limit_extensions='' $f");
2020

2121
echo "Done\n";
2222

‎sapi/cgi/tests/010.phpt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,19 @@ header("HTTP/1.1 403 Forbidden");
1919
header("Status: 403 Also Forbidden");
2020
?>');
2121

22-
echo shell_exec("$php -n $f");
22+
echo shell_exec("$php -n -d cgi.security_limit_extensions='' $f");
2323

2424
file_put_contents($f, '<?php
2525
header("HTTP/1.1 403 Forbidden");
2626
?>');
2727

28-
echo shell_exec("$php -n $f");
28+
echo shell_exec("$php -n -d cgi.security_limit_extensions='' $f");
2929

3030
file_put_contents($f, '<?php
3131
header("Status: 403 Also Forbidden");
3232
?>');
3333

34-
echo shell_exec("$php -n $f");
34+
echo shell_exec("$php -n -d cgi.security_limit_extensions='' $f");
3535

3636
echo "Done\n";
3737

‎sapi/cgi/tests/011.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ $f = tempnam(sys_get_temp_dir(), 'cgitest');
1515
function test($script) {
1616
file_put_contents($GLOBALS['f'], $script);
1717
$cmd = escapeshellcmd($GLOBALS['php']);
18-
$cmd .= ' -n -dreport_zend_debug=0 -dhtml_errors=0 ' . escapeshellarg($GLOBALS['f']);
18+
$cmd .= ' -n -dreport_zend_debug=0 -dhtml_errors=0 -dcgi.security_limit_extensions="" ' . escapeshellarg($GLOBALS['f']);
1919
echo "----------\n";
2020
echo rtrim($script) . "\n";
2121
echo "----------\n";

‎sapi/cgi/tests/012.phpt‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,13 @@ class test
4949

5050
file_put_contents($filename_bad, $code);
5151

52-
run_and_output("$php -n -l $filename_good_escaped $filename_good_escaped");
53-
run_and_output("$php -n -l $filename_good_escaped some.unknown $filename_good_escaped");
54-
run_and_output("$php -n -l $filename_good_escaped $filename_bad_escaped $filename_good_escaped");
55-
run_and_output("$php -n -l $filename_bad_escaped $filename_bad_escaped");
56-
run_and_output("$php -n -l $filename_bad_escaped some.unknown $filename_bad_escaped");
57-
run_and_output("$php -n -l $filename_bad_escaped $filename_bad_escaped some.unknown");
52+
$cmd = "$php -n -d cgi.security_limit_extensions='.test.php .unknown'";
53+
run_and_output("$cmd -l $filename_good_escaped $filename_good_escaped");
54+
run_and_output("$cmd -l $filename_good_escaped some.unknown $filename_good_escaped");
55+
run_and_output("$cmd -l $filename_good_escaped $filename_bad_escaped $filename_good_escaped");
56+
run_and_output("$cmd -l $filename_bad_escaped $filename_bad_escaped");
57+
run_and_output("$cmd -l $filename_bad_escaped some.unknown $filename_bad_escaped");
58+
run_and_output("$cmd -l $filename_bad_escaped $filename_bad_escaped some.unknown");
5859

5960
echo "Done\n";
6061
?>

0 commit comments

Comments
 (0)