88 lines
3.4 KiB
Diff
88 lines
3.4 KiB
Diff
|
From 535eacc96ae6fe5289a2917bb0af43e491b0f4f4 Mon Sep 17 00:00:00 2001
|
||
|
|
From: Panu Matilainen <pmatilai@redhat.com>
|
||
|
|
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 <ffesti@redhat.com>
|
||
|
|
|
||
|
|
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
|
||
|
|
|