Fix GH-21992: symlink() creates link at dangling symlink target

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)

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;
 	}

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--
+<?php
+if (PHP_OS_FAMILY === 'Windows') {
+    die('skip not for Windows');
+}
+?>
+--FILE--
+<?php
+$dir = __DIR__ . '/gh21992';
+$targetDir = $dir . '/target_dir';
+$ghost = $targetDir . '/ghost.txt';
+$deadLink = $dir . '/dead_link.txt';
+$realFile = $dir . '/real.txt';
+
+mkdir($dir);
+mkdir($targetDir);
+touch($realFile);
+
+// Create a dangling link
+var_dump(symlink($ghost, $deadLink));
+var_dump(is_link($deadLink));
+
+echo "--- symlink() over dangling link\n";
+var_dump(symlink($realFile, $deadLink));
+var_dump(readlink($deadLink) === $ghost);
+var_dump(file_exists($ghost) || is_link($ghost));
+
+echo "--- link() over dangling link\n";
+var_dump(link($realFile, $deadLink));
+var_dump(readlink($deadLink) === $ghost);
+var_dump(file_exists($ghost) || is_link($ghost));
+
+echo "--- relative link path\n";
+chdir($dir);
+var_dump(symlink('real.txt', 'dead_link.txt'));
+var_dump(file_exists($ghost) || is_link($ghost));
+
+echo "--- trailing slash\n";
+var_dump(symlink($realFile, $dir . '/slash/'));
+var_dump(file_exists($dir . '/slash') || is_link($dir . '/slash'));
+var_dump(link($realFile, $dir . '/slash/'));
+var_dump(file_exists($dir . '/slash') || is_link($dir . '/slash'));
+var_dump(symlink($realFile, $targetDir . '/'));
+var_dump(link($realFile, $targetDir . '/'));
+?>
+--CLEAN--
+<?php
+$dir = __DIR__ . '/gh21992';
+@unlink($dir . '/target_dir/ghost.txt');
+@unlink($dir . '/dead_link.txt');
+@unlink($dir . '/real.txt');
+@unlink($dir . '/slash');
+@rmdir($dir . '/target_dir');
+@rmdir($dir);
+?>
+--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)

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