Skip to content

Commit

Permalink
Improve error reporting in the fsm
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 other
issues resulting in us setting errno to ENOTDIR. This led to confusing
as the symlink often indeed points at a directory.

Change fsmOpenat to report back an error code and use rpmfilesErrorCodes
from rpmarchive.h - which are already used in the surrounding code.

Give meaningful error message when encountering unsave symlinks.

Retire the abuse of errno for our own reporting.

Resolves: #3100
  • Loading branch information
ffesti committed Aug 14, 2024
1 parent 65fa582 commit 4b19596
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 16 deletions.
37 changes: 24 additions & 13 deletions lib/fsm.c
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ struct filedata_s {
* things around needlessly
*/
static const char * fileActionString(rpmFileAction a);
static int fsmOpenat(int dirfd, const char *path, int flags, int dir);
static int fsmOpenat(int dirfd, const char *path, int flags, int dir, int *rc);
static int fsmClose(int *wfdp);

/** \ingroup payload
Expand Down Expand Up @@ -99,7 +99,7 @@ static int fsmLink(int odirfd, const char *opath, int dirfd, const char *path)
static int cap_set_fileat(int dirfd, const char *path, cap_t fcaps)
{
int rc = -1;
int fd = fsmOpenat(dirfd, path, O_RDONLY|O_NOFOLLOW, 0);
int fd = fsmOpenat(dirfd, path, O_RDONLY|O_NOFOLLOW, 0, NULL);
if (fd >= 0) {
rc = cap_set_fd(fd, fcaps);
fsmClose(&fd);
Expand Down Expand Up @@ -299,12 +299,13 @@ static int fsmMkdir(int dirfd, const char *path, mode_t mode)
return rc;
}

static int fsmOpenat(int dirfd, const char *path, int flags, int dir)
static int fsmOpenat(int dirfd, const char *path, int flags, int dir, int *rcp)
{
struct stat lsb, sb;
int sflags = flags | O_NOFOLLOW;
int fd = openat(dirfd, path, sflags);
int ffd = fd;
int rc = 0;

/*
* Only ever follow symlinks by root or target owner. Since we can't
Expand All @@ -327,11 +328,22 @@ static int fsmOpenat(int dirfd, const char *path, int flags, int dir)
}
}

if (fd < 0)
rc = RPMERR_OPEN_FAILED;

/* O_DIRECTORY equivalent */
if (dir && ((fd != ffd) || (fd >= 0 && fstat(fd, &sb) == 0 && !S_ISDIR(sb.st_mode)))) {
errno = ENOTDIR;
if (dir && fd >= 0 && fstat(fd, &sb) == 0 && !S_ISDIR(sb.st_mode)) {
rc = RPMERR_ENOTDIR;
fsmClose(&fd);
}
/* Symlink with non matching owners */
if (dir && (fd != ffd)) {
rc = RPMERR_INVALID_SYMLINK;
fsmClose(&fd);
}

if (rcp)
*rcp = rc;
return fd;
}

Expand All @@ -351,9 +363,7 @@ static int fsmDoMkDir(rpmPlugins plugins, int dirfd, const char *dn,
rc = fsmMkdir(dirfd, dn, mode);

if (!rc) {
*fdp = fsmOpenat(dirfd, dn, O_RDONLY|O_NOFOLLOW, 1);
if (*fdp == -1)
rc = RPMERR_ENOTDIR;
*fdp = fsmOpenat(dirfd, dn, O_RDONLY|O_NOFOLLOW, 1, &rc);
}

if (!rc) {
Expand Down Expand Up @@ -383,14 +393,14 @@ static int ensureDir(rpmPlugins plugins, const char *p, int owned, int create,
if (*dirfdp >= 0)
return rc;

int dirfd = fsmOpenat(-1, "/", oflags, 1);
int dirfd = fsmOpenat(-1, "/", oflags, 1, NULL);
int fd = dirfd; /* special case of "/" */

char *path = xstrdup(p);
char *dp = path;

while ((bn = strtok_r(dp, "/", &sp)) != NULL) {
fd = fsmOpenat(dirfd, bn, oflags, 1);
fd = fsmOpenat(dirfd, bn, oflags, 1, &rc);
/* assemble absolute path for plugins benefit, sigh */
apath = rstrscat(&apath, "/", bn, NULL);

Expand All @@ -404,10 +414,11 @@ static int ensureDir(rpmPlugins plugins, const char *p, int owned, int create,
dirfd = fd;
} else {
if (!quiet) {
char *msg = rpmfileStrerror(rc);
rpmlog(RPMLOG_ERR, _("failed to open dir %s of %s: %s\n"),
bn, p, strerror(errno));
bn, p, msg);
free(msg);
}
rc = RPMERR_OPEN_FAILED;
break;
}

Expand Down Expand Up @@ -1027,7 +1038,7 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles files,
if (fp->suffix)
flags |= AT_SYMLINK_NOFOLLOW;
fd = fsmOpenat(di.dirfd, fp->fpath, flags,
S_ISDIR(fp->sb.st_mode));
S_ISDIR(fp->sb.st_mode), &rc);
if (fd < 0)
rc = RPMERR_OPEN_FAILED;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/rpmfi.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 = _("Unsave 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/: Not a directory
error: unpacking of archive failed on file /opt/foo;6553448f: cpio: open failed - Not a directory
[error: failed to open dir opt of /opt/: cpio: Unsave symlink
error: unpacking of archive failed on file /opt/foo;6553448f: cpio: Unsave symlink
error: replacetest-1.0-1.noarch: install failed
])
RPMTEST_CLEANUP
Expand Down

0 comments on commit 4b19596

Please sign in to comment.