If the user replaces a directory with a symlink, libalpm would get confused because the trailing slash causes system calls to resolve the symlink. This leads to errors and a misleading message during upgrades. Even though libalpm does not support this, it should not be giving misleading errors. Also adds an overflow check. Fixes FS#51377 Signed-off-by: Andrew Gregory --- See https://bbs.archlinux.org/viewtopic.php?id=224843 for another example. Subtle problems like this could be avoided by removing the trailing slashes from libalpm's internal file lists. Is there any reason not to remove them and use a field in the file_t struct to indicate type? It already has a mode field that we could use. lib/libalpm/remove.c | 13 ++++++++++++- test/pacman/tests/TESTS | 1 + .../tests/remove-directory-replaced-with-symlink.py | 16 ++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 test/pacman/tests/remove-directory-replaced-with-symlink.py diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index 8ce10f84..ffe92518 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -440,8 +440,19 @@ static int unlink_file(alpm_handle_t *handle, alpm_pkg_t *oldpkg, { struct stat buf; char file[PATH_MAX]; + int file_len; - snprintf(file, PATH_MAX, "%s%s", handle->root, fileobj->name); + file_len = snprintf(file, PATH_MAX, "%s%s", handle->root, fileobj->name); + if(file_len <= 0 || file_len >= PATH_MAX) { + /* 0 is a valid value from snprintf, but should be impossible here */ + _alpm_log(handle, ALPM_LOG_DEBUG, "path too long to unlink %s%s\n", + handle->root, fileobj->name); + return -1; + } else if(file[file_len-1] == '/') { + /* trailing slashes cause errors and confusing messages if the user has + * replaced a directory with a symlink */ + file[--file_len] = '\0'; + } if(llstat(file, &buf)) { _alpm_log(handle, ALPM_LOG_DEBUG, "file %s does not exist\n", file); diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS index 11d1c38c..f926e162 100644 --- a/test/pacman/tests/TESTS +++ b/test/pacman/tests/TESTS @@ -110,6 +110,7 @@ TESTS += test/pacman/tests/querycheck002.py TESTS += test/pacman/tests/querycheck_fast_file_type.py TESTS += test/pacman/tests/reason001.py TESTS += test/pacman/tests/remove-assumeinstalled.py +TESTS += test/pacman/tests/remove-directory-replaced-with-symlink.py TESTS += test/pacman/tests/remove-optdepend-of-installed-package.py TESTS += test/pacman/tests/remove-recursive-cycle.py TESTS += test/pacman/tests/remove001.py diff --git a/test/pacman/tests/remove-directory-replaced-with-symlink.py b/test/pacman/tests/remove-directory-replaced-with-symlink.py new file mode 100644 index 00000000..37be7579 --- /dev/null +++ b/test/pacman/tests/remove-directory-replaced-with-symlink.py @@ -0,0 +1,16 @@ +self.description = "remove a package with a directory that has been replaced with a symlink" + +self.filesystem = [ "var/", "srv -> var/" ] + +lpkg = pmpkg("pkg1") +lpkg.files = ["srv/"] +self.addpkg2db("local", lpkg) + +self.args = "-R %s" % (lpkg.name) + +self.addrule("PACMAN_RETCODE=0") +self.addrule("DIR_EXIST=var/") +self.addrule("!LINK_EXIST=srv") +self.addrule("!FILE_EXIST=srv") +self.addrule("!DIR_EXIST=srv") +self.addrule("!PACMAN_OUTPUT=cannot remove") -- 2.12.1