Skip to content

Commit

Permalink
Report unsafe symlinks during installation as a specific case
Browse files Browse the repository at this point in the history
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 <[email protected]>

Resolves: #3100
(cherry picked from commit 535eacc)
  • Loading branch information
pmatilai authored and dmnks committed Aug 30, 2024
1 parent 35f00a2 commit 1f9cac3
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 7 deletions.
10 changes: 6 additions & 4 deletions lib/fsm.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/*
Expand All @@ -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) {
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion lib/rpmfi.c
Original file line number Diff line number Diff line change
Expand Up @@ -2468,7 +2468,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;
Expand Down
4 changes: 2 additions & 2 deletions tests/rpmi.at
Original file line number Diff line number Diff line change
Expand Up @@ -1690,8 +1690,8 @@ runroot --setenv SOURCE_DATE_EPOCH 1699955855 rpm -U /build/RPMS/noarch/replacet
],
[1],
[],
[error: failed to open dir opt of /opt/: cpio: Not a directory
error: unpacking of archive failed on file /opt/foo;6553448f: cpio: Not a directory
[error: failed to open dir opt of /opt/: cpio: Unsafe symlink
error: unpacking of archive failed on file /opt/foo;6553448f: cpio: Unsafe symlink
error: replacetest-1.0-1.noarch: install failed
])
RPMTEST_CLEANUP
Expand Down

0 comments on commit 1f9cac3

Please sign in to comment.