Skip to content

Commit 61e98bf

Browse files
committed
Disallow parent dir components (..) in open_basedir() at runtime
Fix GH-10469 Closes GH-10913
1 parent 439cea4 commit 61e98bf

File tree

4 files changed

+50
-4
lines changed

4 files changed

+50
-4
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ PHP NEWS
2525
. Fix bug GH-10083 (Allow comments between & and parameter). (ilutov)
2626
. Zend Max Execution Timers is now enabled by default for ZTS builds on
2727
Linux. (Kévin Dunglas)
28+
. Fix bug GH-10469 (Disallow .. in open_basedir paths set at runtime).
29+
(ilutov)
2830

2931
- Date:
3032
. Implement More Appropriate Date/Time Exceptions RFC. (Derick)

UPGRADING

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,10 @@ PHP 8.3 UPGRADE NOTES
7474
"buffer_size" => int
7575
See GH-9336
7676
. class_alias() now supports creating an alias of an internal class.
77+
. Setting `open_basedir` at runtime using `ini_set('open_basedir', ...);` no
78+
longer accepts paths containing the parent directory (`..`). Previously,
79+
only paths starting with `..` were disallowed. This could easily be
80+
circumvented by prepending `./` to the path.
7781

7882
- Dom:
7983
. Changed DOMCharacterData::appendData() tentative return type to true.

Zend/tests/gh10469.phpt

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
--TEST--
2+
GH-10469: Disallow open_basedir() with parent dir components (..)
3+
--FILE--
4+
<?php
5+
ini_set('open_basedir', __DIR__);
6+
7+
$originalDir = __DIR__;
8+
$tmpDir = $originalDir . '/gh10469_tmp';
9+
@mkdir($tmpDir, 0777, true);
10+
chdir($tmpDir);
11+
ini_set('open_basedir', ini_get('open_basedir') . ':./..');
12+
ini_set('open_basedir', ini_get('open_basedir') . ':./../');
13+
14+
chdir($originalDir);
15+
var_dump(ini_get('open_basedir'));
16+
?>
17+
--CLEAN--
18+
<?php
19+
@rmdir(__DIR__ . '/gh10469_tmp');
20+
?>
21+
--EXPECTF--
22+
string(%d) "%stests"

main/fopen_wrappers.c

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,11 +101,29 @@ PHPAPI ZEND_INI_MH(OnUpdateBaseDir)
101101
*end = '\0';
102102
end++;
103103
}
104-
if (ptr[0] == '.' && ptr[1] == '.' && (ptr[2] == '\0' || IS_SLASH(ptr[2]))) {
105-
/* Don't allow paths with a leading .. path component to be set at runtime */
106-
efree(pathbuf);
107-
return FAILURE;
104+
/* Don't allow paths with a parent dir component (..) to be set at runtime */
105+
char *substr_pos = ptr;
106+
while (true) {
107+
// Check if we have a .. path component
108+
if (substr_pos[0] == '.'
109+
&& substr_pos[1] == '.'
110+
&& (substr_pos[2] == '\0' || IS_SLASH(substr_pos[2]))) {
111+
efree(pathbuf);
112+
return FAILURE;
113+
}
114+
// Skip to the next path component
115+
while (true) {
116+
substr_pos++;
117+
if (*substr_pos == '\0' || *substr_pos == DEFAULT_DIR_SEPARATOR) {
118+
goto no_parent_dir_component;
119+
} else if (IS_SLASH(*substr_pos)) {
120+
// Also skip the slash
121+
substr_pos++;
122+
break;
123+
}
124+
}
108125
}
126+
no_parent_dir_component:
109127
if (php_check_open_basedir_ex(ptr, 0) != 0) {
110128
/* At least one portion of this open_basedir is less restrictive than the prior one, FAIL */
111129
efree(pathbuf);

0 commit comments

Comments
 (0)