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