From 3c426f8a441a60aad423d0131f66ee8e449b04e9 Mon Sep 17 00:00:00 2001 From: Hilko Bengen Date: Fri, 3 Nov 2023 18:03:53 +0100 Subject: [PATCH] proc/linux: Don't open mmapped files before checking stat results Instead of checking the filesystem containing the mapped file against specific magic device numbers, we use stat(2) before actually opening the file. (The open call can block on certain device nodes as reported in #1929.) To overcome the possible TOCTOU issue between stat() and open(), we double-check device, inode after we have opened the file. This also reverts commit 50eeef4f84f8f5719993a56cc10f1760c1f8dc19. The original problem could also have been fixed by passing O_PATH to open(2). However, there seems to be no way to atomically upgrade those path-only fds to real file descriptors that we can mmap or read from, so this would not helped in overcoming the TOCTOU issue. --- libyara/proc/linux.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/libyara/proc/linux.c b/libyara/proc/linux.c index e1a71308df..813b858bc0 100644 --- a/libyara/proc/linux.c +++ b/libyara/proc/linux.c @@ -161,19 +161,13 @@ YR_API const uint8_t* yr_process_fetch_memory_block_data(YR_MEMORY_BLOCK* block) // Only try mapping the file if it has a path and belongs to a device if (strlen(proc_info->map_path) > 0 && - !(proc_info->map_dmaj == 0 && (proc_info->map_dmin == 0 || proc_info->map_dmin == 5))) + !(proc_info->map_dmaj == 0 && proc_info->map_dmin == 0)) { struct stat st; - fd = open(proc_info->map_path, O_RDONLY); - if (fd < 0) - { - fd = -1; // File does not exist. - } - else if (fstat(fd, &st) < 0) + if (stat(proc_info->map_path, &st) < 0) { // Why should stat fail after file open? Treat like missing. - close(fd); fd = -1; } else if ( @@ -182,22 +176,36 @@ YR_API const uint8_t* yr_process_fetch_memory_block_data(YR_MEMORY_BLOCK* block) (st.st_ino != proc_info->map_ino)) { // Wrong file, may have been replaced. Treat like missing. - close(fd); fd = -1; } else if (st.st_size < proc_info->map_offset + block->size) { // Mapping extends past end of file. Treat like missing. - close(fd); fd = -1; } else if ((st.st_mode & S_IFMT) != S_IFREG) { // Correct filesystem object, but not a regular file. Treat like // uninitialized mapping. - close(fd); fd = -2; } + else + { + fd = open(proc_info->map_path, O_RDONLY); + // Double-check against race conditions + struct stat st2; + if (fstat(fd, &st2) < 0) + { + close(fd); + fd = -1; + } + else if ((st.st_dev != st2.st_dev) || (st.st_ino != st2.st_ino)) + { + // File has been changed from under us, so ignore. + close(fd); + fd = -1; + } + } } if (fd >= 0)