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.

1 files 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 */