From 535eacc96ae6fe5289a2917bb0af43e491b0f4f4 Mon Sep 17 00:00:00 2001 From: Panu Matilainen Date: Mon, 19 Aug 2024 11:43:33 +0300 Subject: [PATCH] Report unsafe symlinks during installation as a specific case Conflict:don't modify testcode because the test code differs greatly, 0091214 requires the root permission to run chown. However, the current test code cannot implement chown. Manual test cases will be used to guard test cases. Reference:https://github.com/rpm-software-management/rpm/commit/535eacc96ae6fe5289a2917bb0af43e491b0f4f4 RPM refuses to follow non root owned symlinks pointing to files owned by another user for security reasons. This case was lumped in with O_DIRECTORY behavior, leading to confusing error message as the symlink often indeed points at a directory. Emit a more meaningful error message when encountering unsafe symlinks. We already detect the error condition in the main if block here, might as well set the error code right there and then so we don't need to redetect later. We previously only tested for the unsafe link condition when our O_DIRECTORY equivalent was set, but that seems wrong. Probably doesn't matter with the existing callers, but we really must not follow those unsafe symlinks no matter what. Co-authored-by: Florian Festi Resolves: #3100 --- lib/fsm.c | 10 ++++++---- lib/rpmfi.c | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/fsm.c b/lib/fsm.c index b5f8800a8..4c0968f73 100644 --- a/lib/fsm.c +++ b/lib/fsm.c @@ -304,7 +304,6 @@ static int fsmOpenat(int *wfdp, int dirfd, const char *path, int flags, int dir) struct stat lsb, sb; int sflags = flags | O_NOFOLLOW; int fd = openat(dirfd, path, sflags); - int ffd = fd; int rc = 0; /* @@ -314,7 +313,7 @@ static int fsmOpenat(int *wfdp, int dirfd, const char *path, int flags, int dir) * it could've only been the link owner or root. */ if (fd < 0 && errno == ELOOP && flags != sflags) { - ffd = openat(dirfd, path, flags); + int ffd = openat(dirfd, path, flags); if (ffd >= 0) { if (fstatat(dirfd, path, &lsb, AT_SYMLINK_NOFOLLOW) == 0) { if (fstat(ffd, &sb) == 0) { @@ -323,13 +322,16 @@ static int fsmOpenat(int *wfdp, int dirfd, const char *path, int flags, int dir) } } } - if (ffd != fd) + /* Symlink with non-matching owners */ + if (ffd != fd) { close(ffd); + rc = RPMERR_INVALID_SYMLINK; + } } } /* O_DIRECTORY equivalent */ - if (!rc && dir && ((fd != ffd) || (fd >= 0 && fstat(fd, &sb) == 0 && !S_ISDIR(sb.st_mode)))) + if (!rc && dir && fd >= 0 && fstat(fd, &sb) == 0 && !S_ISDIR(sb.st_mode)) rc = RPMERR_ENOTDIR; if (!rc && fd < 0) diff --git a/lib/rpmfi.c b/lib/rpmfi.c index a3d8ad470..d8dbb67c7 100644 --- a/lib/rpmfi.c +++ b/lib/rpmfi.c @@ -2426,7 +2426,7 @@ char * rpmfileStrerror(int rc) case RPMERR_DIGEST_MISMATCH: s = _("Digest mismatch"); break; case RPMERR_INTERNAL: s = _("Internal error"); break; case RPMERR_UNMAPPED_FILE: s = _("Archive file not in header"); break; - case RPMERR_INVALID_SYMLINK: s = _("Invalid symlink"); break; + case RPMERR_INVALID_SYMLINK: s = _("Unsafe symlink"); break; case RPMERR_ENOTDIR: s = strerror(ENOTDIR); break; case RPMERR_ENOENT: s = strerror(ENOENT); break; case RPMERR_ENOTEMPTY: s = strerror(ENOTEMPTY); break; -- 2.33.0