From 9184bb0db050d3c551dcfa34ffb42f7bfef5a581 Mon Sep 17 00:00:00 2001 From: Mingjie Shen Date: Thu, 16 Nov 2023 15:34:30 -0500 Subject: [PATCH] ucm: fix TOCTOU race condition Separately checking the state of a file before operating on it may allow an attacker to modify the file between the two operations. Fix by calling readlink first. If that fails, then path should not be a symbolic link and we call open() followed by fstat(). open() with O_NOFOLLOW will return an error if the file is a symlink. Signed-off-by: Mingjie Shen --- src/ucm/ucm_subs.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/ucm/ucm_subs.c b/src/ucm/ucm_subs.c index 2b0103386..a4f97ab78 100644 --- a/src/ucm/ucm_subs.c +++ b/src/ucm/ucm_subs.c @@ -518,30 +518,32 @@ static char *rval_sysfs(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED, const char if (id[0] == '/') id++; snprintf(path, sizeof(path), "%s/%s", e, id); - if (lstat64(path, &sb) != 0) - return NULL; - if (S_ISLNK(sb.st_mode)) { - len = readlink(path, link, sizeof(link) - 1); - if (len <= 0) { - uc_error("sysfs: cannot read link '%s' (%d)", path, errno); - return NULL; - } + len = readlink(path, link, sizeof(link) - 1); + if (len > 0) { link[len] = '\0'; e = strrchr(link, '/'); if (e) return strdup(e + 1); return NULL; } - if (S_ISDIR(sb.st_mode)) - return NULL; - if ((sb.st_mode & S_IRUSR) == 0) - return NULL; - - fd = open(path, O_RDONLY); + fd = open(path, O_RDONLY | O_NOFOLLOW); if (fd < 0) { uc_error("sysfs open failed for '%s' (%d)", path, errno); return NULL; } + if (fstat64(fd, &sb) != 0) { + close(fd); + return NULL; + } + if (S_ISDIR(sb.st_mode)) { + close(fd); + return NULL; + } + if ((sb.st_mode & S_IRUSR) == 0) { + close(fd); + return NULL; + } + len = read(fd, path, sizeof(path)-1); close(fd); if (len < 0) {