Skip to content

Commit

Permalink
Validate intermediate symlinks during installation, CVE-2021-35939
Browse files Browse the repository at this point in the history 
Whenever directory changes during unpacking, walk the entire tree from
starting from / and validate any symlinks crossed, fail the install
on invalid links.

This is the first of step of many towards securing our file operations
against local tamperers and besides plugging that one CVE, paves the way
for the next step by adding the necessary directory fd tracking.
This also bumps the rpm OS requirements to a whole new level by requiring
the *at() family of calls from POSIX-1.2008.

This necessarily does a whole lot of huffing and puffing we previously
did not do. It should be possible to cache secure (ie root-owned)
directory structures to avoid validating everything a million times
but for now, just keeping things simple.
  • Loading branch information
@pmatilai
pmatilai committed Feb 16, 2022
1 parent fb13f7f commit 96ec957
Show file tree
Hide file tree
Showing 3 changed files with 142 additions and 7 deletions.
2 changes: 2 additions & 0 deletions INSTALL
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ option to configure). For GCC, OpenMP 4.5 is fully supported since GCC 6.1,
which is available from
http://www.gnu.org/

Rpm requires a POSIX.1-2008 level operating system.

To compile RPM:
--------------

Expand Down
3 changes: 2 additions & 1 deletion configure.ac
View file
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,8 @@ AC_CHECK_FUNCS([secure_getenv __secure_getenv])

AC_CHECK_FUNCS(
[mkstemp getcwd basename dirname realpath setenv unsetenv regcomp lchown \
utimes getline localtime_r statvfs getaddrinfo ],
utimes getline localtime_r statvfs getaddrinfo \
openat mkdirat fstatat ],
[], [AC_MSG_ERROR([function required by rpm])])

AC_LIBOBJ(fnmatch)
Expand Down
144 changes: 138 additions & 6 deletions lib/fsm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <inttypes.h>
#include <utime.h>
#include <errno.h>
#include <fcntl.h>
#if WITH_CAP
#include <sys/capability.h>
#endif
Expand All @@ -20,6 +21,7 @@
#include "rpmio/rpmio_internal.h" /* fdInit/FiniDigest */
#include "lib/fsm.h"
#include "lib/rpmte_internal.h" /* XXX rpmfs */
#include "lib/rpmfi_internal.h" /* rpmfiSetOnChdir */
#include "lib/rpmplugins.h" /* rpm plugins hooks */
#include "lib/rpmug.h"

Expand Down Expand Up @@ -406,17 +408,118 @@ static int fsmRmdir(const char *path)
return rc;
}

static int fsmMkdir(const char *path, mode_t mode)
static int fsmMkdir(int dirfd, const char *path, mode_t mode)
{
int rc = mkdir(path, (mode & 07777));
int rc = mkdirat(dirfd, path, (mode & 07777));
if (_fsm_debug)
rpmlog(RPMLOG_DEBUG, " %8s (%s, 0%04o) %s\n", __func__,
path, (unsigned)(mode & 07777),
rpmlog(RPMLOG_DEBUG, " %8s (%d %s, 0%04o) %s\n", __func__,
dirfd, path, (unsigned)(mode & 07777),
(rc < 0 ? strerror(errno) : ""));
if (rc < 0) rc = RPMERR_MKDIR_FAILED;
return rc;
}

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

/*
* Only ever follow symlinks by root or target owner. Since we can't
* open the symlink itself, the order matters: we stat the link *after*
* opening the target, and if the link ownership changed between the calls
* it could've only been the link owner or root.
*/
if (fd < 0 && errno == ELOOP && flags != sflags) {
int ffd = openat(dirfd, path, flags);
if (ffd >= 0 && fstatat(dirfd, path, &lsb, AT_SYMLINK_NOFOLLOW) == 0) {
if (fstat(ffd, &sb) == 0) {
if (lsb.st_uid == 0 || lsb.st_uid == sb.st_uid) {
fd = ffd;
} else {
close(ffd);
}
}
}
}
return fd;
}

static int fsmDoMkDir(rpmPlugins plugins, int dirfd, const char *dn,
int owned, mode_t mode)
{
int rc;
rpmFsmOp op = (FA_CREATE);
if (!owned)
op |= FAF_UNOWNED;

/* Run fsm file pre hook for all plugins */
rc = rpmpluginsCallFsmFilePre(plugins, NULL, dn, mode, op);

if (!rc)
rc = fsmMkdir(dirfd, dn, mode);

if (!rc) {
rc = rpmpluginsCallFsmFilePrepare(plugins, NULL, dn, dn, mode, op);
}

/* Run fsm file post hook for all plugins */
rpmpluginsCallFsmFilePost(plugins, NULL, dn, mode, op, rc);

if (!rc) {
rpmlog(RPMLOG_DEBUG,
"%s directory created with perms %04o\n",
dn, (unsigned)(mode & 07777));
}

return rc;
}

static int ensureDir(rpmPlugins plugins, const char *p, int owned, int create)
{
char *path = xstrdup(p);
char *dp = path;
char *sp = NULL, *bn;
int oflags = O_RDONLY;

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

while ((bn = strtok_r(dp, "/", &sp)) != NULL) {
struct stat sb;
fd = fsmOpenat(dirfd, bn, oflags);

if (fd < 0 && errno == ENOENT && create) {
mode_t mode = S_IFDIR | (_dirPerms & 07777);
if (fsmDoMkDir(plugins, dirfd, bn, owned, mode) == 0) {
fd = fsmOpenat(dirfd, bn, oflags|O_NOFOLLOW);
}
}

if (fd >= 0 && fstat(fd, &sb) == 0 && !S_ISDIR(sb.st_mode)) {
close(fd);
errno = ENOTDIR;
fd = -1;
}

close(dirfd);
if (fd >= 0) {
dirfd = fd;
} else {
dirfd = -1;
rpmlog(RPMLOG_ERR, _("failed to open dir %s of %s: %s\n"),
bn, p, strerror(errno));
break;
}

dp = NULL;
}

free(path);
return dirfd;
}

static int fsmMkfifo(const char *path, mode_t mode)
{
int rc = mkfifo(path, (mode & 07777));
Expand Down Expand Up @@ -507,7 +610,7 @@ static int fsmMkdirs(rpmfiles files, rpmfs fs, rpmPlugins plugins)
rc = rpmpluginsCallFsmFilePre(plugins, NULL, dn, mode, op);

if (!rc)
rc = fsmMkdir(dn, mode);
rc = fsmMkdir(-1, dn, mode);

if (!rc) {
rc = rpmpluginsCallFsmFilePrepare(plugins, NULL, dn, dn,
Expand Down Expand Up @@ -874,6 +977,21 @@ static void setFileState(rpmfs fs, int i)
}
}

struct diriter_s {
int dirfd;
};

static int onChdir(rpmfi fi, void *data)
{
struct diriter_s *di = data;

if (di->dirfd >= 0) {
close(di->dirfd);
di->dirfd = -1;
}
return 0;
}

int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles files,
rpmpsm psm, char ** failedFile)
{
Expand All @@ -890,6 +1008,7 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles files,
char *tid = NULL;
struct filedata_s *fdata = xcalloc(fc, sizeof(*fdata));
struct filedata_s *firstlink = NULL;
struct diriter_s di = { -1 };

/* transaction id used for temporary path suffix while installing */
rasprintf(&tid, ";%08x", (unsigned)rpmtsGetTid(ts));
Expand Down Expand Up @@ -932,6 +1051,7 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles files,
rc = RPMERR_BAD_MAGIC;
goto exit;
}
rpmfiSetOnChdir(fi, onChdir, &di);

/* Detect and create directories not explicitly in package. */
if (!rc)
Expand All @@ -946,6 +1066,16 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles files,
if (!fp->suffix) {
rc = fsmBackup(fi, fp->action);
}

if (di.dirfd == -1) {
di.dirfd = ensureDir(plugins, rpmfiDN(fi), 0,
(fp->action == FA_CREATE));
if (di.dirfd == -1) {
rc = RPMERR_OPEN_FAILED;
break;
}
}

/* Assume file does't exist when tmp suffix is in use */
if (!fp->suffix) {
if (fp->action == FA_TOUCH) {
Expand Down Expand Up @@ -980,7 +1110,7 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles files,
mode_t mode = fp->sb.st_mode;
mode &= ~07777;
mode |= 00700;
rc = fsmMkdir(fp->fpath, mode);
rc = fsmMkdir(di.dirfd, fp->fpath, mode);
}
} else if (S_ISLNK(fp->sb.st_mode)) {
if (rc == RPMERR_ENOENT) {
Expand Down Expand Up @@ -1022,6 +1152,8 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles files,
fp->stage = FILE_UNPACK;
}
fi = rpmfiFree(fi);
close(di.dirfd);
di.dirfd = -1;

if (!rc && fx < 0 && fx != RPMERR_ITER_END)
rc = fx;
Expand Down

0 comments on commit 96ec957

Please sign in to comment.