From 5345ad63a175a3d6487a48471aa0edadaf10e440 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Sat, 10 Oct 2026 22:01:13 +0200 Subject: [PATCH] Fix GH-21992: symlink() creates link at dangling symlink target expand_filepath() resolves the last component of the link path, so when it is an existing dangling symlink, the new link is created at its target instead of failing. The same applies to link(). Resolve only the directory part and keep the last component as given so the kernel reports EEXIST. --- NEWS | 4 + ext/standard/link.c | 59 +++++++++++++-- ext/standard/tests/file/gh21992.phpt | 92 +++++++++++++++++++++++ tests/security/open_basedir_readlink.phpt | 2 +- 4 files changed, 151 insertions(+), 6 deletions(-) create mode 100644 ext/standard/tests/file/gh21992.phpt diff --git a/NEWS b/NEWS index ee8ab9f3529c..37d01c825ae2 100644 --- a/NEWS +++ b/NEWS @@ -37,6 +37,10 @@ PHP NEWS . Fixed incorrect handling of non-ASCII pending_input bytes in readline_info(), which could cause readline() to return false prematurely. (Weilin Du) +- Standard: + . Fixed bug GH-21992 (symlink() on dangling symlink returns true and creates + link at wrong location). (Jakub Zelenka) + - Streams: . Fixed bug GH-22063 (Stream filter chain UAF on self-removal during callback). (Ilia Alshanetsky) diff --git a/ext/standard/link.c b/ext/standard/link.c index df2831b87846..5e15935b0f4e 100644 --- a/ext/standard/link.c +++ b/ext/standard/link.c @@ -120,6 +120,56 @@ PHP_FUNCTION(linkinfo) } /* }}} */ +/* Expand the link path but do not resolve its last component (it is the link itself) */ +static char *php_expand_link_path(const char *path, size_t path_len, char *expanded, char *dir_p, size_t *dir_len) +{ + char dir[MAXPATHLEN]; + const char *base_end, *base; + size_t len, base_len; + + if (path_len == 0 || path_len >= MAXPATHLEN) { + return NULL; + } + + memcpy(dir, path, path_len + 1); + zend_dirname(dir, path_len); + if (!expand_filepath(dir, dir_p)) { + return NULL; + } + len = strlen(dir_p); + + /* Find the last component, ignoring trailing slashes */ + base_end = path + path_len; + while (base_end > path && IS_SLASH(base_end[-1])) { + base_end--; + } + base = base_end; + while (base > path && !IS_SLASH(base[-1])) { + base--; + } + base_len = base_end - base; + + /* dir + slash + base + optional trailing slash + NUL */ + if (len + 1 + base_len + 1 >= MAXPATHLEN) { + return NULL; + } + + memcpy(expanded, dir_p, len); + *dir_len = len; + if (!IS_SLASH(dir_p[len - 1])) { + expanded[len++] = DEFAULT_SLASH; + } + memcpy(expanded + len, base, base_len); + len += base_len; + /* Keep a trailing slash so that the kernel rejects a non-directory target the same way as before */ + if (base_end < path + path_len) { + expanded[len++] = DEFAULT_SLASH; + } + expanded[len] = '\0'; + + return expanded; +} + /* {{{ Create a symbolic link */ PHP_FUNCTION(symlink) { @@ -136,14 +186,11 @@ PHP_FUNCTION(symlink) Z_PARAM_PATH(frompath, frompath_len) ZEND_PARSE_PARAMETERS_END(); - if (!expand_filepath(frompath, source_p)) { + if (!php_expand_link_path(frompath, frompath_len, source_p, dirname, &len)) { php_error_docref(NULL, E_WARNING, "No such file or directory"); RETURN_FALSE; } - memcpy(dirname, source_p, sizeof(source_p)); - len = zend_dirname(dirname, strlen(dirname)); - if (!expand_filepath_ex(topath, dest_p, dirname, len)) { php_error_docref(NULL, E_WARNING, "No such file or directory"); RETURN_FALSE; @@ -186,13 +233,15 @@ PHP_FUNCTION(link) int ret; char source_p[MAXPATHLEN]; char dest_p[MAXPATHLEN]; + char dirname[MAXPATHLEN]; + size_t len; ZEND_PARSE_PARAMETERS_START(2, 2) Z_PARAM_PATH(topath, topath_len) Z_PARAM_PATH(frompath, frompath_len) ZEND_PARSE_PARAMETERS_END(); - if (!expand_filepath(frompath, source_p) || !expand_filepath(topath, dest_p)) { + if (!php_expand_link_path(frompath, frompath_len, source_p, dirname, &len) || !expand_filepath(topath, dest_p)) { php_error_docref(NULL, E_WARNING, "No such file or directory"); RETURN_FALSE; } diff --git a/ext/standard/tests/file/gh21992.phpt b/ext/standard/tests/file/gh21992.phpt new file mode 100644 index 000000000000..b70b7df44113 --- /dev/null +++ b/ext/standard/tests/file/gh21992.phpt @@ -0,0 +1,92 @@ +--TEST-- +GH-21992 (symlink() on dangling symlink returns true and creates link at wrong location) +--SKIPIF-- + +--FILE-- + +--CLEAN-- + +--EXPECTF-- +bool(true) +bool(true) +--- symlink() over dangling link + +Warning: symlink(): File exists in %s on line %d +bool(false) +bool(true) +bool(false) +--- link() over dangling link + +Warning: link(): File exists in %s on line %d +bool(false) +bool(true) +bool(false) +--- relative link path + +Warning: symlink(): File exists in %s on line %d +bool(false) +bool(false) +--- trailing slash + +Warning: symlink(): No such file or directory in %s on line %d +bool(false) +bool(false) + +Warning: link(): No such file or directory in %s on line %d +bool(false) +bool(false) + +Warning: symlink(): File exists in %s on line %d +bool(false) + +Warning: link(): File exists in %s on line %d +bool(false) diff --git a/tests/security/open_basedir_readlink.phpt b/tests/security/open_basedir_readlink.phpt index 34a0f18f4bf8..575179041cbc 100644 --- a/tests/security/open_basedir_readlink.phpt +++ b/tests/security/open_basedir_readlink.phpt @@ -65,7 +65,7 @@ bool(false) Warning: readlink(): open_basedir restriction in effect. File(%s/test/ok/symlink.txt) is not within the allowed path(s): (%sok) in %s on line %d bool(false) -Warning: symlink(): open_basedir restriction in effect. File(%s/test/bad/bad.txt) is not within the allowed path(s): (%sok) in %s on line %d +Warning: symlink(): open_basedir restriction in effect. File(%s/test/ok/symlink.txt) is not within the allowed path(s): (%sok) in %s on line %d bool(false) Warning: readlink(): open_basedir restriction in effect. File(%s/test/ok/symlink.txt) is not within the allowed path(s): (%sok) in %s on line %d