git: 17dc7017d737 - main - install: Simplify path construction.
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Fri, 12 Apr 2024 17:31:47 UTC
The branch main has been updated by des: URL: https://cgit.FreeBSD.org/src/commit/?id=17dc7017d7375b3463d65adffe1eb980b0f86795 commit 17dc7017d7375b3463d65adffe1eb980b0f86795 Author: Dag-Erling Smørgrav <des@FreeBSD.org> AuthorDate: 2024-04-12 17:30:52 +0000 Commit: Dag-Erling Smørgrav <des@FreeBSD.org> CommitDate: 2024-04-12 17:31:35 +0000 install: Simplify path construction. There's no need to copy the path twice to split it into base and dir. We simply call `basename()` first, then handle the two trivial cases in which it isn't safe to call `dirname()`. While here, add an early check that the destination is not an empty string. This would always fail eventually, so it may as well fail right away. Also add a test case for this shortcut. MFC after: 1 week Sponsored by: Klara, Inc. Reviewed by: markj Differential Revision: https://reviews.freebsd.org/D44743 --- usr.bin/xinstall/tests/install_test.sh | 8 ++++++++ usr.bin/xinstall/xinstall.c | 34 +++++++++++++++++++++------------- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/usr.bin/xinstall/tests/install_test.sh b/usr.bin/xinstall/tests/install_test.sh index cbe95f0dfbf3..b35706521ec3 100755 --- a/usr.bin/xinstall/tests/install_test.sh +++ b/usr.bin/xinstall/tests/install_test.sh @@ -25,6 +25,13 @@ # # +atf_test_case copy_to_empty +copy_to_empty_body() { + printf 'test\n123\r456\r\n789\0z' >testf + atf_check -s not-exit:0 -e match:"empty string" \ + install testf "" +} + copy_to_nonexistent_with_opts() { printf 'test\n123\r456\r\n789\0z' >testf atf_check install "$@" testf copyf @@ -497,6 +504,7 @@ set_optional_exec_body() } atf_init_test_cases() { + atf_add_test_case copy_to_empty atf_add_test_case copy_to_nonexistent atf_add_test_case copy_to_nonexistent_safe atf_add_test_case copy_to_nonexistent_comparing diff --git a/usr.bin/xinstall/xinstall.c b/usr.bin/xinstall/xinstall.c index 6d2f05d64e2c..6ab0a88d5cd7 100644 --- a/usr.bin/xinstall/xinstall.c +++ b/usr.bin/xinstall/xinstall.c @@ -657,8 +657,10 @@ static void makelink(const char *from_name, const char *to_name, const struct stat *target_sb) { - char src[MAXPATHLEN], dst[MAXPATHLEN], lnk[MAXPATHLEN]; - struct stat to_sb; + char src[MAXPATHLEN], dst[MAXPATHLEN], lnk[MAXPATHLEN]; + char *to_name_copy, *d, *ld, *ls, *s; + const char *base, *dir; + struct stat to_sb; /* Try hard links first. */ if (dolink & (LN_HARD|LN_MIXED)) { @@ -719,8 +721,6 @@ makelink(const char *from_name, const char *to_name, } if (dolink & LN_RELATIVE) { - char *to_name_copy, *cp, *d, *ld, *ls, *s; - if (*from_name != '/') { /* this is already a relative link */ do_symlink(from_name, to_name, target_sb); @@ -740,17 +740,23 @@ makelink(const char *from_name, const char *to_name, to_name_copy = strdup(to_name); if (to_name_copy == NULL) err(EX_OSERR, "%s: strdup", to_name); - cp = dirname(to_name_copy); - if (realpath(cp, dst) == NULL) - err(EX_OSERR, "%s: realpath", cp); - /* .. and add the last component. */ - if (strcmp(dst, "/") != 0) { - if (strlcat(dst, "/", sizeof(dst)) > sizeof(dst)) + base = basename(to_name_copy); + if (base == to_name_copy) { + /* destination is a file in cwd */ + (void)strlcpy(dst, "./", sizeof(dst)); + } else if (base == to_name_copy + 1) { + /* destination is a file in the root */ + (void)strlcpy(dst, "/", sizeof(dst)); + } else { + /* all other cases: safe to call dirname() */ + dir = dirname(to_name_copy); + if (realpath(dir, dst) == NULL) + err(EX_OSERR, "%s: realpath", dir); + if (strcmp(dst, "/") != 0 && + strlcat(dst, "/", sizeof(dst)) > sizeof(dst)) errx(1, "resolved pathname too long"); } - strcpy(to_name_copy, to_name); - cp = basename(to_name_copy); - if (strlcat(dst, cp, sizeof(dst)) > sizeof(dst)) + if (strlcat(dst, base, sizeof(dst)) > sizeof(dst)) errx(1, "resolved pathname too long"); free(to_name_copy); @@ -834,6 +840,8 @@ install(const char *from_name, const char *to_name, u_long fset, u_int flags) } else { devnull = 1; } + if (*to_name == '\0') + errx(EX_USAGE, "destination cannot be an empty string"); target = (lstat(to_name, &to_sb) == 0);