From 41498dc454025f55184a9182b967b92fb7e6f643 Mon Sep 17 00:00:00 2001 From: kcgen Date: Sat, 22 Oct 2022 14:02:45 -0700 Subject: Fix a time-of-check time-of-use (TOCTOU) scenario in ManyMouse (CWE-367) TOCTOU security issues exists between a check event and a use event in which an attacker can change properties about the thing being used. In this case, the code checked several stat-based properties about the mouse's character-device file path, and once verified, opened the the file path as a bona fide mouse device. Because the same open file descriptor isn't maintained across the checks through to the usage, we can't be guaranteed that file used is the one we checked (the crux of the TOCTOU vulnerability). To fix it, we hold the sample file descriptor across the checks and usage. --- src/libs/manymouse/linux_evdev.c | 46 +++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/src/libs/manymouse/linux_evdev.c b/src/libs/manymouse/linux_evdev.c index 6d9b58185..c99cd2257 100644 --- a/src/libs/manymouse/linux_evdev.c +++ b/src/libs/manymouse/linux_evdev.c @@ -7,6 +7,7 @@ * Altered to: * - silence compiler warnings, by Roman Standzikowski. * - silence compiler warnings, by kcgen. + * - fix a time-of-check time-of-use issue, by kcgen. */ #include "manymouse.h" @@ -220,33 +221,48 @@ static int init_mouse(__attribute__((unused)) const char *fname, int fd) } /* init_mouse */ -/* Return a file descriptor if this is really a mouse, -1 otherwise. */ +/* Return 1 if this is really a mouse, 0 otherwise. */ static int open_if_mouse(const char *fname) { + assert(fname); + + /* Can we open the file device? */ + const int fd = open(fname, O_RDONLY | O_NONBLOCK); + if (fd == -1) + return 0; + struct stat statbuf; - int fd; - int devmajor, devminor; + memset(&statbuf, 0, sizeof(statbuf)); - if (stat(fname, &statbuf) == -1) + /* Can we stat info about the device? */ + if (fstat(fd, &statbuf) == -1) { + close(fd); return 0; + } - if (S_ISCHR(statbuf.st_mode) == 0) - return 0; /* not a character device... */ + /* Is it a character device? */ + if (S_ISCHR(statbuf.st_mode) == 0) { + close(fd); + return 0; + } /* evdev node ids are major 13, minor 64-96. Is this safe to check? */ - devmajor = (statbuf.st_rdev & 0xFF00) >> 8; - devminor = (statbuf.st_rdev & 0x00FF); - if ( (devmajor != 13) || (devminor < 64) || (devminor > 96) ) + const int devmajor = (statbuf.st_rdev & 0xFF00) >> 8; + const int devminor = (statbuf.st_rdev & 0x00FF); + if ( (devmajor != 13) || (devminor < 64) || (devminor > 96) ) { + close(fd); return 0; /* not an evdev. */ + } - if ((fd = open(fname, O_RDONLY | O_NONBLOCK)) == -1) + /* Were we able to initialize the evdev mouse handle? */ + const char * unused_filename = NULL; + if (init_mouse(unused_filename, fd) != 1) { + close(fd); return 0; + } - if (init_mouse(fname, fd)) - return 1; - - close(fd); - return 0; + assert(fd >= 0); + return 1; } /* open_if_mouse */ -- cgit v1.2.3