commit b7422a2f5c775615ff397e14c04aa9c9a41504c1
from: Stefan Sperling <stsp@stsp.name>
date: Thu Jul 23 14:22:38 2020 UTC

stop using realpath(3) to resolve a symlink target in install_symlink()

We should not resolve a symlink target path recursively when installing a
symlink in the work tree. We want to handle this symlink's target, not the
end result of following a chain of symlinks in case such links already exist.

commit - 369fd7e5fa99b95f7d7aa812b5260584b86a3778
commit + b7422a2f5c775615ff397e14c04aa9c9a41504c1
blob - 2a2d317c015385526ce0f22c09ed3240dafad6a5
blob + cec70da8d017f316d73c553fe1f13cb015f3d6e8
--- lib/worktree.c
+++ lib/worktree.c
@@ -1255,8 +1255,8 @@ install_symlink(int *is_bad_symlink, struct got_worktr
 {
 	const struct got_error *err = NULL;
 	char target_path[PATH_MAX];
+	char canonpath[PATH_MAX];
 	size_t len, target_len = 0;
-	char *resolved_path = NULL, *abspath = NULL;
 	char *path_got = NULL;
 	const uint8_t *buf = got_object_blob_get_read_buf(blob);
 	size_t hdrlen = got_object_blob_get_hdrlen(blob);
@@ -1292,10 +1292,14 @@ install_symlink(int *is_bad_symlink, struct got_worktr
 	target_path[target_len] = '\0';
 
 	/*
+	 * We do not use realpath(3) to resolve the symlink's target path
+	 * because we don't want to resolve symlinks recursively. Instead
+	 * we make the path absolute and then canonicalize it.
 	 * Relative symlink target lookup should begin at the directory
 	 * in which the blob object is being installed.
 	 */
 	if (!got_path_is_absolute(target_path)) {
+		char *abspath;
 		char *parent = dirname(ondisk_path);
 		if (parent == NULL) {
 			err = got_error_from_errno2("dirname", ondisk_path);
@@ -1305,24 +1309,18 @@ install_symlink(int *is_bad_symlink, struct got_worktr
 			err = got_error_from_errno("asprintf");
 			goto done;
 		}
-	}
-
-	/*
-	 * unveil(2) restricts our view of paths in the filesystem.
-	 * ENOENT will occur if a link target path does not exist or
-	 * if it points outside our unveiled path space.
-	 */
-	resolved_path = realpath(abspath ? abspath : target_path, NULL);
-	if (resolved_path == NULL) {
-		if (errno != ENOENT) {
-			err = got_error_from_errno2("realpath", target_path);
+		err = got_canonpath(abspath, canonpath, sizeof(canonpath));
+		free(abspath);
+		if (err)
 			goto done;
-		}
+	} else {
+		err = got_canonpath(target_path, canonpath, sizeof(canonpath));
+		if (err)
+			goto done;
 	}
 
 	/* Only allow symlinks pointing at paths within the work tree. */
-	if (!got_path_is_child(resolved_path ? resolved_path : (abspath ?
-	    abspath : target_path), worktree->root_path,
+	if (!got_path_is_child(canonpath, worktree->root_path,
 	    strlen(worktree->root_path))) {
 		/* install as a regular file */
 		*is_bad_symlink = 1;
@@ -1340,8 +1338,7 @@ install_symlink(int *is_bad_symlink, struct got_worktr
 		err = got_error_from_errno("asprintf");
 		goto done;
 	}
-	if (got_path_is_child(resolved_path ? resolved_path : (abspath ?
-	    abspath : target_path), path_got, strlen(path_got))) {
+	if (got_path_is_child(canonpath, path_got, strlen(path_got))) {
 		/* install as a regular file */
 		*is_bad_symlink = 1;
 		got_object_blob_rewind(blob);
@@ -1413,8 +1410,6 @@ install_symlink(int *is_bad_symlink, struct got_worktr
 		err = (*progress_cb)(progress_arg, reverting_versioned_file ?
 		    GOT_STATUS_REVERT : GOT_STATUS_ADD, path);
 done:
-	free(resolved_path);
-	free(abspath);
 	free(path_got);
 	return err;
 }
blob - 4ed5d4635a5d84136d563642565378542947f5fc
blob + b0bd8dbc81e9a03e974842f00623c4174ba9cef8
--- regress/cmdline/checkout.sh
+++ regress/cmdline/checkout.sh
@@ -509,19 +509,42 @@ function test_checkout_symlink {
 	(cd $testroot/repo && ln -s alpha alpha.link)
 	(cd $testroot/repo && ln -s epsilon epsilon.link)
 	(cd $testroot/repo && ln -s /etc/passwd passwd.link)
+	(cd $testroot/repo && ln -s passwd.link passwd2.link)
 	(cd $testroot/repo && ln -s ../beta epsilon/beta.link)
 	(cd $testroot/repo && ln -s nonexistent nonexistent.link)
 	(cd $testroot/repo && ln -s .got/foo dotgotfoo.link)
 	(cd $testroot/repo && git add .)
 	git_commit $testroot/repo -m "add symlinks"
 
-	got checkout $testroot/repo $testroot/wt > /dev/null
+	got checkout $testroot/repo $testroot/wt > $testroot/stdout
 	ret="$?"
 	if [ "$ret" != "0" ]; then
+		echo "got checkout failed unexpectedly" >&2
 		test_done "$testroot" "$ret"
 		return 1
 	fi
+
+	echo "A  $testroot/wt/alpha" > $testroot/stdout.expected
+	echo "A  $testroot/wt/alpha.link" >> $testroot/stdout.expected
+	echo "A  $testroot/wt/beta" >> $testroot/stdout.expected
+	echo "A  $testroot/wt/dotgotfoo.link" >> $testroot/stdout.expected
+	echo "A  $testroot/wt/epsilon/beta.link" >> $testroot/stdout.expected
+	echo "A  $testroot/wt/epsilon/zeta" >> $testroot/stdout.expected
+	echo "A  $testroot/wt/epsilon.link" >> $testroot/stdout.expected
+	echo "A  $testroot/wt/gamma/delta" >> $testroot/stdout.expected
+	echo "A  $testroot/wt/nonexistent.link" >> $testroot/stdout.expected
+	echo "A  $testroot/wt/passwd.link" >> $testroot/stdout.expected
+	echo "A  $testroot/wt/passwd2.link" >> $testroot/stdout.expected
+	echo "Now shut up and hack" >> $testroot/stdout.expected
 
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
 	if ! [ -h $testroot/wt/alpha.link ]; then
 		echo "alpha.link is not a symlink"
 		test_done "$testroot" "1"
@@ -572,6 +595,22 @@ function test_checkout_symlink {
 		return 1
 	fi
 
+	if ! [ -h $testroot/wt/passwd2.link ]; then
+		echo "passwd2.link is not a symlink"
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	readlink $testroot/wt/passwd2.link > $testroot/stdout
+	echo "passwd.link" > $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
 	readlink $testroot/wt/epsilon/beta.link > $testroot/stdout
 	echo "../beta" > $testroot/stdout.expected
 	cmp -s $testroot/stdout.expected $testroot/stdout