From 1731c6b3ef1e486ab1156d10701242c4a6899a35 Mon Sep 17 00:00:00 2001
From: Pierre Le Marre <dev@wismill.eu>
Date: Mon, 5 Feb 2024 11:55:39 +0100
Subject: [PATCH] compose: Ensure we mmap only regular files

Currently we do not check that the Compose files we try successively are
*regular* files. This may result in an error, while we should just really
just skip the corresponding path.

Fixed by adding the new utily function `open_file`.
---
 src/compose/table.c | 32 ++++++++++++--------------------
 src/utils.c         | 36 ++++++++++++++++++++++++++++++++++--
 src/utils.h         |  6 ++++++
 test/compose.c      | 13 +++++++++++++
 4 files changed, 65 insertions(+), 22 deletions(-)

diff --git a/src/compose/table.c b/src/compose/table.c
index 254db0b..8fa5951 100644
--- a/src/compose/table.c
+++ b/src/compose/table.c
@@ -176,35 +176,27 @@ xkb_compose_table_new_from_locale(struct xkb_context *ctx,
         return NULL;
 
     path = get_xcomposefile_path(ctx);
-    if (path) {
-        file = fopen(path, "rb");
-        if (file)
-            goto found_path;
-    }
+    file = open_file(path);
+    if (file)
+        goto found_path;
     free(path);
 
     path = get_xdg_xcompose_file_path(ctx);
-    if (path) {
-        file = fopen(path, "rb");
-        if (file)
-            goto found_path;
-    }
+    file = open_file(path);
+    if (file)
+        goto found_path;
     free(path);
 
     path = get_home_xcompose_file_path(ctx);
-    if (path) {
-        file = fopen(path, "rb");
-        if (file)
-            goto found_path;
-    }
+    file = open_file(path);
+    if (file)
+        goto found_path;
     free(path);
 
     path = get_locale_compose_file_path(ctx, table->locale);
-    if (path) {
-        file = fopen(path, "rb");
-        if (file)
-            goto found_path;
-    }
+    file = open_file(path);
+    if (file)
+        goto found_path;
     free(path);
 
     log_err(ctx, XKB_LOG_MESSAGE_NO_ID,
diff --git a/src/utils.c b/src/utils.c
index dbb0662..66a8c0a 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -23,14 +23,14 @@
 
 #include "config.h"
 
+#include <sys/stat.h>
+#include <fcntl.h>
 #include "utils.h"
 
 #ifdef HAVE_MMAP
 
-#include <fcntl.h>
 #include <unistd.h>
 #include <sys/mman.h>
-#include <sys/stat.h>
 #include <sys/types.h>
 
 bool
@@ -111,6 +111,38 @@ unmap_file(char *str, size_t size)
 
 #endif
 
+/* Open a file and ensure it is a regular file.
+ * Returns NULL in case of error. */
+FILE*
+open_file(const char *path)
+{
+    if (!path)
+        return NULL;
+
+    int fd = open(path, O_RDONLY);
+    if (fd < 0)
+        return NULL;
+
+    struct stat stat_buf;
+    int err = fstat(fd, &stat_buf);
+
+    if (err != 0 || !S_ISREG(stat_buf.st_mode)) {
+        close(fd);
+        return NULL;
+    }
+
+    /* While unlikely to happen, if `fp` is NULL then we must close the file
+     * descriptor. This is poorly documented and missing from the man page; see instead:
+     * https://www.ibm.com/docs/en/i/7.3?topic=functions-fdopen-associates-stream-file-descriptor)
+     * Confirmed by Peter to be necessary in glibc. */
+    FILE *fp = fdopen(fd, "rb");
+    if (fp == NULL) {
+        close(fd);
+    }
+
+    return fp;
+}
+
 // ASCII lower-case map.
 static const unsigned char lower_map[] = {
     0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20,
diff --git a/src/utils.h b/src/utils.h
index 95deba1..9d44b85 100644
--- a/src/utils.h
+++ b/src/utils.h
@@ -47,6 +47,9 @@
 # ifndef S_ISDIR
 #  define S_ISDIR(m) (((m) & S_IFMT) == S_IFDIR)
 # endif
+# ifndef S_ISREG
+#  define S_ISREG(m) (((m) & S_IFMT) == S_IFREG)
+# endif
 typedef SSIZE_T ssize_t;
 #endif
 
@@ -263,6 +266,9 @@ check_eaccess(const char *path, int mode)
     return true;
 }
 
+FILE*
+open_file(const char *path);
+
 #if defined(HAVE_SECURE_GETENV)
 # define secure_getenv secure_getenv
 #elif defined(HAVE___SECURE_GETENV)
diff --git a/test/compose.c b/test/compose.c
index 50ac40a..d6c88e0 100644
--- a/test/compose.c
+++ b/test/compose.c
@@ -23,6 +23,7 @@
 
 #include "config.h"
 #include <time.h>
+#include <errno.h>
 
 #include "xkbcommon/xkbcommon-compose.h"
 
@@ -499,6 +500,18 @@ test_XCOMPOSEFILE(struct xkb_context *ctx)
     struct xkb_compose_table *table;
     char *path;
 
+    /* Error: directory */
+    path = test_get_path("locale/en_US.UTF-8");
+    setenv("XCOMPOSEFILE", path, 1);
+    free(path);
+
+    table = xkb_compose_table_new_from_locale(ctx, "blabla",
+                                              XKB_COMPOSE_COMPILE_NO_FLAGS);
+    assert_printf(errno != ENODEV && errno != EISDIR,
+                  "Should not be an error from `map_file`\n");
+    assert(!table);
+
+    /* OK: regular file */
     path = test_get_path("locale/en_US.UTF-8/Compose");
     setenv("XCOMPOSEFILE", path, 1);
     free(path);