From 6d7ba8b54612e3ed4aed706cec8e18e4fde907e1 Mon Sep 17 00:00:00 2001 From: CBL-Mariner-Bot <75509084+CBL-Mariner-Bot@users.noreply.github.com> Date: Wed, 9 Oct 2024 16:50:18 -0400 Subject: [PATCH] [AUTO-CHERRYPICK] Fix CVE-2024-47191 in oath-toolkit - branch main (#10650) Co-authored-by: Mandeep Plaha <99760213+mandeepsplaha@users.noreply.github.com> --- SPECS/oath-toolkit/CVE-2024-47191.patch | 873 ++++++++++++++++++++++++ SPECS/oath-toolkit/oath-toolkit.spec | 13 +- 2 files changed, 881 insertions(+), 5 deletions(-) create mode 100644 SPECS/oath-toolkit/CVE-2024-47191.patch diff --git a/SPECS/oath-toolkit/CVE-2024-47191.patch b/SPECS/oath-toolkit/CVE-2024-47191.patch new file mode 100644 index 00000000000..e2addd65682 --- /dev/null +++ b/SPECS/oath-toolkit/CVE-2024-47191.patch @@ -0,0 +1,873 @@ +From 4302b149a186ba8ca155ea7e211c25fac112a3ef Mon Sep 17 00:00:00 2001 +From: Matthias Gerstner +Date: Wed, 11 Sep 2024 14:09:25 +0200 +Subject: [PATCH] usersfile: fix potential security issues in PAM module + context (CVE-2024-47191) + +With the addition of the possibility to place a usersfile also into +a user's home directory via variable expansion of ${HOME} and ${USER} in +the `usersfile=` path specification, security issues sneaked in. The PAM +process usually runs with root privileges. The file operations in an +unprivileged user's home directory follow symlinks both when reading and +creating files, allowing for a potential local root exploit, because of +the `fchown()` performed on the newly created usersfile. + +The situation is not that easy to fix, since the current PAM module +configuration does not indicate explicitly whether the usersfile will be +placed in an unprivileged or in a privileged location. It is advisable +to drop privileges to the owner of the usersfile, if we're running as +root. To determine the ownership of the usersfile, it first has to be +opened in a safe way, though. + +This change addresses the issue by introducing a usersfile_ctx datatype +which holds state information about the target usersfile. The new +function `safe_open_usersfile()` will open the target path in a safe +way, rejecting any symlinks on the way. The function also rejects any +world-writable directories or files, which would generally be a bad idea +to have in the usersfile path. + +The global `umask()` alteration is dropped in favor of using an unnamed +temporary file to achieve the proper file permissions of a newly created +usersfile. Since the target mode is 0600, the umask would need to be +really awkward anyway to change the outcome. `fchown()` is no longer +called on the new file, assuming we are already running with the correct +credentials. + +The locking logic of the existing code is incomplete, because the +initial reading of the usersfile is performed without locking. Only +during updating of the file, the lock is obtained. I believe this can +lead to inconsistencies. Also the current code unlinks the lockfile +after its use, which opens a race condition making the lock again +unreliable. + +The creation of the lockfile in the directory containing the usersfile +is somewhat unfortunate. Lockfiles are runtime state data that should go +into /run or a shared sticky-bit directory. It is unclear whether mixed +root and non-root accesses need to be synchronized (probably). An +advantage of using the location of the usersfile is that if the +usersfile should be placed on a network share (NFS, CIFS), that the +locking can theoretically happen across the network. + +This patch aims to make the locking complete by acquiring it before +parsing the actual usersfile. To prevent cluttering of users' home +directories no separate lockfile is used anymore, but the usersfile +itself it used for locking. This involves some extra complexity, since +even after acquiring the lock, the actual usersfile on disk might have +been replaced by a new one in the meantime. This situation needs to be +detected and recovered from. + +In the PAM module context the unprivileged user could try to DoS the +privileged PAM stack, by taking the lock and never releasing it. +Therefore a polling loop is implemented that fails after 15 seconds of +failing to obtain the lock. Unfortunately there exists no lock with +timeout API, thus it needs to be polled. + +Instead of the POSIX compatible fcntl(F_SETLK) locking API this patch +switches to the Linux specific fcntl(F_OFD_SETLK) locking. The reason +for this is that locks obtained with F_SETLK cannot be inherited to +child processes, which we need to do now. flock() would also have been +an alternative, but it has unfortunate properties if the lockfile should +be located on a network file system. +--- + +diff -Naur oath-toolkit-2.6.7-mariner-patched/liboath/errors.c oath-toolkit-2.6.7/liboath/errors.c +--- oath-toolkit-2.6.7-mariner-patched/liboath/errors.c 2024-10-05 19:53:03.559981287 -0700 ++++ oath-toolkit-2.6.7/liboath/errors.c 2024-10-05 19:21:23.755488969 -0700 +@@ -58,7 +58,12 @@ + ERR (OATH_FILE_SYNC_ERROR, "System error when syncing file to disk"), + ERR (OATH_FILE_CLOSE_ERROR, "System error when closing file"), + ERR (OATH_FILE_CHOWN_ERROR, "System error when changing file ownership"), +- ERR (OATH_FILE_STAT_ERROR, "System error when getting file status") ++ ERR (OATH_FILE_STAT_ERROR, "System error when getting file status"), ++ ERR (OATH_FILE_OPEN_ERROR, "System error trying to open file"), ++ ERR (OATH_FORK_ERROR, "System error when forking a process"), ++ ERR (OATH_WAIT_ERROR, "System error when waiting for a process"), ++ ERR (OATH_SETUID_ERROR, "System error when setting process UID"), ++ ERR (OATH_SETGID_ERROR, "System error when setting process GID") + }; + + /** +diff -Naur oath-toolkit-2.6.7-mariner-patched/liboath/oath.h.in oath-toolkit-2.6.7/liboath/oath.h.in +--- oath-toolkit-2.6.7-mariner-patched/liboath/oath.h.in 2024-10-05 19:53:03.939985058 -0700 ++++ oath-toolkit-2.6.7/liboath/oath.h.in 2024-10-05 19:21:56.115570760 -0700 +@@ -152,9 +152,14 @@ + OATH_FILE_CLOSE_ERROR = -25, + OATH_FILE_CHOWN_ERROR = -26, + OATH_FILE_STAT_ERROR = -27, ++ OATH_FILE_OPEN_ERROR = -28, ++ OATH_FORK_ERROR = -29, ++ OATH_WAIT_ERROR = -30, ++ OATH_SETUID_ERROR = -31, ++ OATH_SETGID_ERROR = -32, + /* When adding anything here, update OATH_LAST_ERROR, errors.c + and tests/tst_errors.c. */ +- OATH_LAST_ERROR = -27 ++ OATH_LAST_ERROR = -33 + } oath_rc; + + /* Global */ +diff -Naur oath-toolkit-2.6.7-mariner-patched/liboath/usersfile.c oath-toolkit-2.6.7/liboath/usersfile.c +--- oath-toolkit-2.6.7-mariner-patched/liboath/usersfile.c 2024-10-05 19:55:00.017139982 -0700 ++++ oath-toolkit-2.6.7/liboath/usersfile.c 2024-10-05 19:37:06.910860525 -0700 +@@ -29,7 +29,226 @@ + #include /* For ssize_t. */ + #include /* For fcntl. */ + #include /* For errno. */ ++#include /* For PATH_MAX & friends. */ + #include /* For S_IRUSR, S_IWUSR. */ ++#include /* For wait */ ++#include /* For stat */ ++ ++struct usersfile_ctx { ++ const char *path; ++ const char *basename; /* basename of path, points into `path` */ ++ int parent_fd; /* file descriptor for the parent directory of the usersfile */ ++ int fd; /* file descriptor for the usersfile */ ++ struct stat st; /* stat information for the usersfile */ ++}; ++ ++/* ++ * Upgrade a file descriptor opened with O_PATH to a fully functional file ++ * descriptor. ++ * ++ * To achieve this the file is reopened via /proc, which is supported by the ++ * Linux kernel. `fd` needs to point to the currently open file descriptor. On ++ * success it will be replaced by the new upgraded file descriptor, while the ++ * original file descriptor will be closed. ++ * ++ * `flags` are passed to `open()` for the new file descriptor. ++ */ ++static int ++reopen_path_fd (int *fd, int flags) ++{ ++ /* we need to open /proc/self/fd/, so the path won't get too long here */ ++ char proc_path[128]; ++ int res = snprintf(proc_path, sizeof(proc_path), "/proc/self/fd/%d", *fd); ++ ++ if (res < 0 || res >= sizeof(proc_path)) ++ return OATH_PRINTF_ERROR; ++ ++ int newfd = open(proc_path, flags); ++ ++ if (newfd < 0) ++ return OATH_FILE_OPEN_ERROR; ++ ++ close(*fd); ++ *fd = newfd; ++ return OATH_OK; ++} ++ ++static void ++init_usersfile_ctx(struct usersfile_ctx *ctx, const char *path) ++{ ++ ctx->path = path; ++ ctx->basename = NULL; ++ ctx->parent_fd = -1; ++ ctx->fd = -1; ++ memset(&ctx->st, 0, sizeof(ctx->st)); ++} ++ ++static void ++destroy_usersfile_ctx(struct usersfile_ctx *ctx) ++{ ++ if (ctx->parent_fd != -1) ++ { ++ close (ctx->parent_fd); ++ ctx->parent_fd = -1; ++ } ++ ++ if (ctx->fd != -1) ++ { ++ close (ctx->fd); ++ ctx->fd = -1; ++ } ++ ++ /* reset everything but keep the path so it might be reused */ ++ init_usersfile_ctx(ctx, ctx->path); ++} ++ ++/* ++ * Obtain a lock for the usersfile. The lock is placed on the usersfile itself ++ * as found in `ctx->fd` ++ * ++ * On success the lock on `ctx->fd` has been correctly obtained. ++ */ ++static int ++lock_usersfile (struct usersfile_ctx *ctx) ++{ ++ /* ++ * There exist three file locking APIs: ++ * ++ * - flock(): this would be the simplest API, but it doesn't properly support ++ * network file systems like NFS, which then causes a transparent fallback ++ * to fcntl() file locking. ++ * - fcntl using F_SETLCK & friends: this lock is not based on the open file ++ * description and thus cannot be inherited to child processes, which we ++ * need to do. ++ * - fcntl using F_OFD_SETLCK & friends: this is a Linux specific lock that ++ * _is_ based on the open file description. It seems like the best bet for ++ * our scenario. ++ * ++ * Since we are potentially running in PAM module context, we have to ++ * take a local DoS scenario into account here, where the unprivileged user ++ * holds the lock, preventing us from ever getting it. ++ * ++ * There's no file locking API supporting a timeout (except for using a ++ * SIGALRM timer to interrupt the system call). Using asynchronous signals ++ * in a library is not so great. Thus make a best effort polling attempt: ++ * ++ * `F_OFD_SETLK` polls for the lock. If we cannot get it, sleep half a ++ * second and retry. Do this for at max 15 seconds, else fail. ++ */ ++ ++ struct flock fl; ++ memset(&fl, 0, sizeof(fl)); ++ /* lock the entire file with a write lock */ ++ fl.l_type = F_WRLCK; ++ fl.l_whence = SEEK_SET; ++ fl.l_start = 0; ++ fl.l_len = 0; ++ ++ for (int i = 0; i < 30; i++) { ++ if (fcntl(ctx->fd, F_OFD_SETLK, &fl) == 0) ++ return OATH_OK; ++ ++ if (errno == EACCES || errno == EAGAIN) ++ usleep(1000 * 500); ++ else ++ break; ++ } ++ ++ return OATH_FILE_LOCK_ERROR; ++} ++ ++/* ++ * After traversing all directory path elements this function actually opens ++ * the target usersfile. `ctx->parent_fd` must be valid. ++ * ++ * This function takes care of the locking logic, which is a bit complicated, ++ * since we use the usersfile itself for locking. This is done, because we ++ * don't want to clutter arbitrary directories with lockfiles, possibly making ++ * the locking also less robust (e.g. if users delete them interactively). ++ * ++ * Since we don't actually write to the usersfile, but replace it atomically, ++ * to prevent any inconsistent state to ever be stored to disk, we need a ++ * recovery mechanism if we obtain a lock on the file, but the file has ++ * already been replaced by a new version. This situation is detected by ++ * opening the file again after the lock has been placed and comparing the ++ * inode numbers. If the no longer match, then the new file has to be locked ++ * instead. ++ * ++ * On successful return ctx->fd will be valid and locked and ctx->st will ++ * contain the current stat information for the usersfile. ++ */ ++static int ++finish_open_usersfile (struct usersfile_ctx *ctx) ++{ ++ const int oflags = O_RDONLY|O_PATH|O_CLOEXEC|O_NOFOLLOW; ++ ctx->fd = openat(ctx->parent_fd, ctx->basename, oflags); ++ ++ if (ctx->fd < 0) ++ return errno == ENOENT ? OATH_NO_SUCH_FILE : OATH_FILE_OPEN_ERROR; ++ ++ if (fstat(ctx->fd, &ctx->st) != 0) ++ return OATH_FILE_STAT_ERROR; ++ ++ /* lock and retry opening until all is consistent, abort after a couple of ++ * times, it's unlikely that we race all the time (could be a DoS attempt) */ ++ for (int i = 0; i < 5; i++) ++ { ++ /* deny world-writable or special usersfile */ ++ if ((ctx->st.st_mode & S_IWOTH) != 0 || !S_ISREG(ctx->st.st_mode)) ++ return OATH_FILE_OPEN_ERROR; ++ ++ /* we need to open it read-write for write-locking it via fcntl(), ++ * otherwise we wouldn't need write access for the file, since we'll ++ * atomically replace it with a new one. */ ++ int err = reopen_path_fd(&ctx->fd, O_RDWR|O_CLOEXEC); ++ if (err != OATH_OK) ++ return err; ++ ++ err = lock_usersfile(ctx); ++ if (err != OATH_OK) ++ return err; ++ ++ /* ++ * we now own a lock on the usersfile, but another process might already ++ * have replaced the file in question by new version. Thus we need to ++ * check whether the file is still there and is the same as the one we ++ * have opened. Otherwise a race occurred an we need to retry. ++ */ ++ int check_fd = openat(ctx->parent_fd, ctx->basename, oflags); ++ struct stat check_st; ++ err = fstat(check_fd, &check_st); ++ if (err != OATH_OK) ++ { ++ close(check_fd); ++ return err; ++ } ++ ++ /* comparing the inode should be enough, since parent_fd didn't change, ++ * so it should be the same file system */ ++ if (ctx->st.st_ino != check_st.st_ino) ++ { ++ /* race occurred, retry using the new FD */ ++ close(ctx->fd); ++ ctx->fd = check_fd; ++ memcpy(&ctx->st, &check_st, sizeof(ctx->st)); ++ continue; ++ } ++ ++ /* we own the lock and the file is still in place, we did it */ ++ close(check_fd); ++ ++ /* now also reopen the parent directory FD, so it can be used for ++ * fsync() later on. */ ++ err = reopen_path_fd(&ctx->parent_fd, O_RDONLY|O_CLOEXEC|O_DIRECTORY); ++ if (err != OATH_OK) ++ return err; ++ ++ return OATH_OK; ++ } ++ ++ /* maximum number of locking attempts exceeded */ ++ return OATH_FILE_LOCK_ERROR; ++} + + static int + parse_type (const char *str, unsigned *digits, unsigned *totpstepsize) +@@ -296,8 +515,92 @@ + return OATH_OK; + } + ++/* ++ * create a new file in the directory referred to by ctx->parent_fd. A unique ++ * filename will be selected and written out to `newname`. ++ */ + static int +-update_usersfile (const char *usersfile, ++create_new_usersfile(struct usersfile_ctx *ctx, char *newname) ++{ ++ int err = OATH_OK; ++ newname[0] = '\0'; ++ ++ /* create an unnamed temporary file, this way we can fix the file mode ++ without anybody else being able to access the file */ ++ int fd = openat(ctx->parent_fd, ".", O_TMPFILE|O_WRONLY|O_CLOEXEC, 0600); ++ if (fd < 0) ++ return OATH_FILE_OPEN_ERROR; ++ ++ /* make sure the mode is as we want it, since umask might have changed the outcome. */ ++ if (fchmod(fd, 0600) != 0) ++ { ++ err = OATH_FILE_CHOWN_ERROR; ++ goto out; ++ } ++ ++ /* there's nothing like mkostmpat() where we can use our parent_fd. ++ * tmpname() & friends are deprecated and also not fully suitable here. ++ * ++ * what we're actually missing here is an additional flag LINKAT_REPLACE ++ * which would allow to atomically replace the original file, instead of ++ * using renameat(). This doesn't exist yet, though. ++ * ++ * linkat() doesn't follow symlinks or overwrite files, so we're safe here ++ * against any shenanigans. The user owning parent_fd can try to guess the ++ * filename we're using here and thus DoS us. Setup an arbitrary limit of ++ * creation attempts to prevent an infinite loop in such situations. Such a ++ * bad actor would then only DoS itself, preventing login. ++ * ++ * Shared world-writable directories should never be used for the usersfile, ++ * this would be a configuration error, thus we don't try to protect against ++ * such scenarios. ++ * ++ * An alternative would be using rand(), but then we'd need to also seed it, ++ * with possible process wide side effects, which is also not great. ++ */ ++ ++ int ret = snprintf(newname, NAME_MAX, "%s.new.%d", ctx->basename, getpid()); ++ if (ret < 0 || ret >= NAME_MAX) ++ { ++ err = OATH_PRINTF_ERROR; ++ goto out; ++ } ++ ++ /* we need to specify /proc/self/fd/, so the path won't get too long here */ ++ char proc_path[128]; ++ ret = snprintf(proc_path, sizeof(proc_path), "/proc/self/fd/%d", fd); ++ if (ret < 0 || ret >= NAME_MAX) ++ { ++ err = OATH_PRINTF_ERROR; ++ goto out; ++ } ++ ++ /* we cannot reliably use AT_EMPTY_PATH here, since it can require the ++ * CAP_DAC_READ_SEARCH capability when running as non-root. Starting with ++ * kernel 6.10 this requirement has been softened, but we need to stay ++ * backward compatible. Linking the magic link in /proc into the directory ++ * works without extra capabilities. ++ * For this workaround to function AT_SYMLINK_FOLLOW _must_ be specified ++ * so this is a conscious decision. ++ */ ++ if (linkat(AT_FDCWD, proc_path, ctx->parent_fd, newname, AT_SYMLINK_FOLLOW)) ++ { ++ err = OATH_FILE_CREATE_ERROR; ++ } ++ ++out: ++ if (err != OATH_OK) ++ { ++ if (fd >= 0) ++ close(fd); ++ return err; ++ } ++ ++ return fd; ++} ++ ++static int ++update_usersfile (struct usersfile_ctx *ctx, + const char *username, + const char *otp, + FILE * infh, +@@ -305,9 +608,7 @@ + size_t *n, char *timestamp, uint64_t new_moving_factor, + size_t skipped_users) + { +- FILE *outfh, *lockfh; + int rc; +- char *newfilename, *lockfile; + + /* Rewind input file. */ + { +@@ -319,120 +620,236 @@ + clearerr (infh); + } + +- /* Open lockfile. */ +- { +- int l; ++ char newfilename[NAME_MAX]; + +- if (oath_lockfile_path) ++ /* Open the "new" file. We aim for atomic replacement of the old file to ++ * address possible power failure or system lockup scenarios. */ ++ int outfd = create_new_usersfile(ctx, newfilename); ++ if (outfd < 0) + { +- l = asprintf (&lockfile, "%s", oath_lockfile_path); +- if (lockfile == NULL || ((size_t) l) != strlen (oath_lockfile_path)) +- return OATH_PRINTF_ERROR; ++ return outfd; + } +- else ++ ++ FILE *outfh = fdopen (outfd, "w"); ++ if (!outfh) + { +- l = asprintf (&lockfile, "%s.lock", usersfile); +- if (lockfile == NULL || ((size_t) l) != strlen (usersfile) + 5) +- return OATH_PRINTF_ERROR; ++ rc = OATH_FILE_CREATE_ERROR; ++ goto out; + } + +- lockfh = fopen (lockfile, "w"); +- if (!lockfh) +- { +- free (lockfile); +- return OATH_FILE_CREATE_ERROR; +- } +- } ++ /* ownership has been transferred to outfh */ ++ outfd = -1; + +- /* Lock the lockfile. */ +- { +- struct flock l; ++ /* Create the new usersfile content. */ ++ rc = update_usersfile2 (username, otp, infh, outfh, lineptr, n, ++ timestamp, new_moving_factor, skipped_users); + +- memset (&l, 0, sizeof (l)); +- l.l_whence = SEEK_SET; +- l.l_start = 0; +- l.l_len = 0; +- l.l_type = F_WRLCK; ++ if (rc != OATH_OK) ++ goto out; + +- while ((rc = fcntl (fileno (lockfh), F_SETLKW, &l)) < 0 && errno == EINTR) +- continue; +- if (rc == -1) +- { +- fclose (lockfh); +- free (lockfile); +- return OATH_FILE_LOCK_ERROR; +- } ++ /* On success, flush the buffers. */ ++ if (fflush (outfh) != 0) { ++ rc = OATH_FILE_FLUSH_ERROR; ++ goto out; + } + +- /* Open the "new" file. */ +- { +- int l; +- +- l = asprintf (&newfilename, "%s.new", usersfile); +- if (newfilename == NULL || ((size_t) l) != strlen (usersfile) + 4) +- { +- fclose (lockfh); +- free (lockfile); +- return OATH_PRINTF_ERROR; +- } ++ /* On success, sync the disks. */ ++ if (fsync (fileno (outfh)) != 0) { ++ rc = OATH_FILE_SYNC_ERROR; ++ goto out; ++ } + +- outfh = fopen (newfilename, "w"); +- if (!outfh) +- { +- free (newfilename); +- fclose (lockfh); +- free (lockfile); +- return OATH_FILE_CREATE_ERROR; +- } ++ /* On success, replace the usersfile with the new copy. ++ * This does not follow symlinks in the target, the target will always be ++ * replaced. ++ * */ ++ if (renameat (ctx->parent_fd, newfilename, ctx->parent_fd, ctx->basename) != 0) { ++ rc = OATH_FILE_RENAME_ERROR; ++ goto out; + } + +- /* Create the new usersfile content. */ +- rc = update_usersfile2 (username, otp, infh, outfh, lineptr, n, +- timestamp, new_moving_factor, skipped_users); ++ /* this name no longer exists now */ ++ newfilename[0] = '\0'; + +- /* Preserve ownership of the new usersfile file */ +- { +- struct stat insb; ++ /* make sure the directory is also synced such that directory inodes are written out */ ++ if (fsync(ctx->parent_fd) != 0) { ++ rc = OATH_FILE_SYNC_ERROR; ++ goto out; ++ } ++ ++out: ++ if (outfd >= 0) ++ close(outfd); ++ if (outfh) ++ fclose(outfh); ++ if (rc != OATH_OK && newfilename[0]) ++ unlinkat(ctx->parent_fd, newfilename, 0); ++ return rc; ++} + +- if(rc == OATH_OK && fstat(fileno(infh), &insb) == -1) +- rc = OATH_FILE_STAT_ERROR; ++static int ++oath_process_usersfile (struct usersfile_ctx *ctx, ++ const char *username, ++ const char *otp, ++ size_t window, ++ const char *passwd, time_t *last_otp) ++{ ++ FILE *infh; ++ char *line = NULL; ++ size_t n = 0; ++ uint64_t new_moving_factor; ++ int rc; ++ size_t skipped_users; + +- if(rc == OATH_OK && fchown(fileno(outfh), insb.st_uid, insb.st_gid) != 0) +- rc = OATH_FILE_CHOWN_ERROR; +- } ++ infh = fdopen (ctx->fd, "r"); ++ if (infh == NULL) ++ return OATH_FILE_OPEN_ERROR; + +- /* On success, flush the buffers. */ +- if (rc == OATH_OK && fflush (outfh) != 0) +- rc = OATH_FILE_FLUSH_ERROR; ++ /* ownership has been transferred to the FILE stream now */ ++ ctx->fd = -1; + +- /* On success, sync the disks. */ +- if (rc == OATH_OK && fsync (fileno (outfh)) != 0) +- rc = OATH_FILE_SYNC_ERROR; ++ rc = parse_usersfile (username, otp, window, passwd, last_otp, ++ infh, &line, &n, &new_moving_factor, &skipped_users); ++ ++ if (rc == OATH_OK) ++ { ++ char timestamp[30]; ++ size_t max = sizeof (timestamp); ++ struct tm now; ++ time_t t; ++ size_t l; + +- /* Close the file regardless of success. */ +- if (fclose (outfh) != 0) +- rc = OATH_FILE_CLOSE_ERROR; ++ if (time (&t) == (time_t) - 1) ++ return OATH_TIME_ERROR; + +- /* On success, overwrite the usersfile with the new copy. */ +- if (rc == OATH_OK && rename (newfilename, usersfile) != 0) +- rc = OATH_FILE_RENAME_ERROR; ++ if (localtime_r (&t, &now) == NULL) ++ return OATH_TIME_ERROR; + +- /* Something has failed, don't leave garbage lying around. */ +- if (rc != OATH_OK) +- unlink (newfilename); ++ l = strftime (timestamp, max, TIME_FORMAT_STRING, &now); ++ if (l != 20) ++ return OATH_TIME_ERROR; + +- free (newfilename); ++ rc = update_usersfile (ctx, username, otp, infh, ++ &line, &n, timestamp, new_moving_factor, ++ skipped_users); ++ } + +- /* Complete, close the lockfile */ +- if (fclose (lockfh) != 0) +- rc = OATH_FILE_CLOSE_ERROR; +- if (unlink (lockfile) != 0) +- rc = OATH_FILE_UNLINK_ERROR; +- free (lockfile); ++ free (line); ++ fclose (infh); + + return rc; + } + ++/* ++ * Safely open `ctx->path`, filling all the other fields in `ctx` from it. On ++ * error destroy_usersfile_ctx() is invoked for `ctx`. ++ * ++ * When operating with raised privileges we cannot know the ownership of ++ * `ctx->path` in advance, thus we need to carefully open the path. Any ++ * symbolic links in the path will be rejected for simplicity reasons. ++ * ++ * Every path element will be extracted step-by-step and opened by passing the ++ * `O_PATH` flag. This is the safest approach which prevents any side effects ++ * that might result from opening e.g. FIFO special files, symlinks or device ++ * files. ++ * ++ * Once the final path element has been reached and verified, the file ++ * descriptors have to be upgraded to regular ones without the `O_PATH` ++ * property, for being able to use them for regular file operations. ++ * ++ * NOTE: a similar result can be achieved by using openat2() and passing ++ * RESOLVE_NO_SYMLINKS, but the system call is not yet wrapped in Glibc, which ++ * makes it hard to use it. ++ */ ++static int ++safe_open_usersfile (struct usersfile_ctx *ctx) ++{ ++ int err = OATH_OK; ++ ++ /* reject relative paths */ ++ if (ctx->path[0] != '/') ++ return OATH_FILE_OPEN_ERROR; ++ ++ ctx->parent_fd = open("/", O_PATH|O_DIRECTORY|O_CLOEXEC|O_RDONLY); ++ if (ctx->parent_fd < 0) ++ return OATH_FILE_OPEN_ERROR; ++ ++ char *path_start = strdup (ctx->path); ++ if (!path_start) { ++ err = OATH_MALLOC_ERROR; ++ goto out; ++ } ++ ++ char *element = path_start; ++ ++ while (true) ++ { ++ /* ignore any extra leading slashes */ ++ while (element[0] == '/') ++ element++; ++ ++ /* end of path has been reached (trailing slashes? shouldn't really happen) */ ++ if (!element[0]) ++ { ++ err = OATH_FILE_OPEN_ERROR; ++ goto out; ++ } ++ ++ char *sep = strpbrk(element, "/"); ++ ++ /* intermediate path (directory) element */ ++ if (sep) ++ { ++ *sep = '\0'; ++ ++ ctx->fd = openat(ctx->parent_fd, element, O_RDONLY|O_PATH|O_CLOEXEC|O_NOFOLLOW|O_DIRECTORY); ++ ++ if (ctx->fd < 0) ++ { ++ err = errno == ENOENT ? OATH_NO_SUCH_FILE : OATH_FILE_OPEN_ERROR; ++ goto out; ++ } ++ ++ if (fstat(ctx->fd, &ctx->st) != 0) ++ { ++ err = OATH_FILE_STAT_ERROR; ++ goto out; ++ } ++ ++ /* If we encounter any world-writable components, refuse the path. ++ * This prevents any unwise configurations like placing the file into ++ * /var/tmp or a dedicated world-writable sticky-bit directory from ++ * working. */ ++ if (ctx->st.st_mode & S_IWOTH) ++ { ++ err = OATH_FILE_OPEN_ERROR; ++ goto out; ++ } ++ ++ close(ctx->parent_fd); ++ ctx->parent_fd = ctx->fd; ++ ctx->fd = -1; ++ element = sep + 1; ++ } ++ /* final path element has been encountered */ ++ else ++ { ++ ctx->basename = ctx->path + (element - path_start); ++ err = finish_open_usersfile(ctx); ++ break; ++ } ++ } ++ ++ ++out: ++ if (err != OATH_OK) ++ { ++ destroy_usersfile_ctx(ctx); ++ } ++ free (path_start); ++ return err; ++} ++ + /** + * oath_authenticate_usersfile: + * @usersfile: string with user credential filename, in UsersFile format +@@ -466,50 +883,67 @@ + size_t window, + const char *passwd, time_t * last_otp) + { +- FILE *infh; +- char *line = NULL; +- size_t n = 0; +- uint64_t new_moving_factor; + int rc; +- size_t skipped_users; +- +- infh = fopen (usersfile, "r"); +- if (!infh) +- return OATH_NO_SUCH_FILE; ++ struct usersfile_ctx ctx; ++ init_usersfile_ctx(&ctx, usersfile); + +- rc = parse_usersfile (username, otp, window, passwd, last_otp, +- infh, &line, &n, &new_moving_factor, &skipped_users); +- +- if (rc == OATH_OK) ++ rc = safe_open_usersfile (&ctx); ++ if (rc < 0) ++ return rc; ++ ++ /* if user is not root we cannot change credentials, ++ just run _oath_authenticate_usersfile normally in this case. ++ Similarly if the file is owned by root, we don't need to change ++ credentials. */ ++ if (geteuid () != 0 || ctx.st.st_uid == 0) ++ { ++ rc = oath_process_usersfile (&ctx, username, otp, window, passwd, last_otp); ++ destroy_usersfile_ctx(&ctx); ++ return rc; ++ } ++ ++ /* else spawn a new process so we can drop privileges to the owner of the ++ * file, to be on the safe side when operating in a directory owned by ++ * non-root. */ ++ pid_t cpid = fork (); ++ if (cpid < 0) + { +- char timestamp[30]; +- size_t max = sizeof (timestamp); +- struct tm now; +- time_t t; +- size_t l; +- mode_t old_umask; +- +- if (time (&t) == (time_t) - 1) +- return OATH_TIME_ERROR; +- +- if (localtime_r (&t, &now) == NULL) +- return OATH_TIME_ERROR; +- +- l = strftime (timestamp, max, TIME_FORMAT_STRING, &now); +- if (l != 20) +- return OATH_TIME_ERROR; +- +- old_umask = umask (~(S_IRUSR | S_IWUSR)); +- +- rc = update_usersfile (usersfile, username, otp, infh, +- &line, &n, timestamp, new_moving_factor, +- skipped_users); +- +- umask (old_umask); ++ destroy_usersfile_ctx(&ctx); ++ return OATH_FORK_ERROR; + } + +- free (line); +- fclose (infh); ++ if (cpid == 0) ++ { ++ /* child */ ++ if (setgid (ctx.st.st_gid) != 0) ++ exit (abs(OATH_SETGID_ERROR)); ++ if (setuid (ctx.st.st_uid) != 0) ++ exit (abs(OATH_SETUID_ERROR)); ++ rc = oath_process_usersfile (&ctx, username, otp, window, passwd, last_otp); ++ exit (abs(rc)); ++ } ++ else ++ { ++ int status; ++ rc = waitpid (cpid, &status, 0); ++ if (rc < 0) ++ goto wait_out; + +- return rc; ++ if (!WIFEXITED(status)) ++ { ++ /* child exited abnormally */ ++ rc = OATH_WAIT_ERROR; ++ goto wait_out; ++ } ++ ++ const int exit_code = WEXITSTATUS(status); ++ rc = exit_code == 0 ? OATH_OK : -exit_code; ++wait_out: ++ /* ++ * only destroy the ctx after the child exited, otherwise the lockfile ++ * would be unlinked before the job is finished. ++ */ ++ destroy_usersfile_ctx(&ctx); ++ return rc; ++ } + } diff --git a/SPECS/oath-toolkit/oath-toolkit.spec b/SPECS/oath-toolkit/oath-toolkit.spec index 43c0d9c4717..61bad6d3520 100644 --- a/SPECS/oath-toolkit/oath-toolkit.spec +++ b/SPECS/oath-toolkit/oath-toolkit.spec @@ -1,14 +1,15 @@ Summary: One-time password components Name: oath-toolkit Version: 2.6.7 -Release: 2%{?dist} +Release: 3%{?dist} License: GPLv3+ and LGPLv2+ URL: https://www.nongnu.org/oath-toolkit/ Vendor: Microsoft Corporation Distribution: Mariner Source0: https://download.savannah.gnu.org/releases/%{name}/%{name}-%{version}.tar.gz -Patch0: oath-toolkit-2.6.2-lockfile.patch +Patch0: oath-toolkit-2.6.2-lockfile.patch +Patch1: CVE-2024-47191.patch BuildRequires: pam-devel BuildRequires: gtk-doc @@ -110,8 +111,7 @@ Requires: pam A PAM module for pluggable login authentication for OATH. %prep -%setup -q -%patch0 -p1 -b .lockfile +%autosetup -p1 %build autoreconf -fi @@ -186,6 +186,9 @@ mkdir -p -m 0600 %{buildroot}%{_sysconfdir}/liboath %{_libdir}/security/pam_oath.so %changelog +* Thu Oct 03 2024 Mandeep Plaha - 2.6.7-3 +- Fix CVE-2024-47191 + * Wed Sep 20 2023 Jon Slobodzian - 2.6.7-2 - Recompile with stack-protection fixed gcc version (CVE-2023-4039) @@ -297,4 +300,4 @@ mkdir -p -m 0600 %{buildroot}%{_sysconfdir}/liboath - Added /etc/liboath directory to hold configuration / user lists * Sun Apr 07 2013 Jaroslav Škarvada - 2.0.2-1 -- Initial version \ No newline at end of file +- Initial version