From c1e6bb6b03dfd934b33b91ab4102c0c8778a0ff0 Mon Sep 17 00:00:00 2001 From: xujin Date: Thu, 23 Jul 2026 14:07:23 +0800 Subject: [PATCH] fix: add FIFO validation to prevent arbitrary file writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added `sys/stat.h` include and FIFO type verification for file descriptors and paths used in FIFO-based interprocess communication. The fix ensures only valid FIFO files are opened for writing, preventing security vulnerabilities where arbitrary file paths could be used to write to unintended locations. This addresses potential issues with `--notify-fifo` and `--fd1`/`--fd2` arguments where the provided path or file descriptor might not be a FIFO, mitigating the risk of unauthorized file writes. Influence: 1. Test FIFO notification system with valid FIFO paths 2. Verify FIFO validation with regular files instead of FIFOs 3. Test with non-existent paths to ensure proper error handling 4. Verify security loader helper arguments (`--fd1`, `--fd2`) with valid FIFOs 5. Test with invalid file types (sockets, regular files) passed as FIFO arguments 6. Verify error logging and graceful handling when invalid paths/fds are detected fix: 添加FIFO验证以防止任意文件写入 添加了`sys/stat.h`头文件，并对FIFO通信中使用的文件描述符和路径进行了FIFO 类型验证。此修复确保只打开有效的FIFO文件进行写入，防止因提供任意文件路径 而写入未预期位置的潜在安全漏洞。解决了`--notify-fifo`和`--fd1`/`--fd2`参 数可能指向非FIFO文件的问题，降低了未经授权文件写入的风险。 Influence: 1. 使用有效的FIFO路径测试FIFO通知系统 2. 使用普通文件替代FIFO验证FIFO类型检查 3. 测试不存在的路径以确保正确处理错误 4. 使用有效FIFO验证安全加载器助手参数(`--fd1`, `--fd2`) 5. 测试将无效文件类型（套接字、普通文件）作为FIFO参数传递的情况 6. 验证检测到无效路径/文件描述符时的错误记录和优雅处理 PMS: TASK-393165 --- src/dde-update/main.cpp | 19 ++++++++++++++++++- src/dde-update/securityloaderhelper.cpp | 17 +++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/dde-update/main.cpp b/src/dde-update/main.cpp index e6ff7306..ea366292 100644 --- a/src/dde-update/main.cpp +++ b/src/dde-update/main.cpp @@ -24,6 +24,7 @@ #include #include #include +#include Q_LOGGING_CATEGORY(logUpdateModal, "dde.update.modalupdate") @@ -86,12 +87,20 @@ class FifoNotifier if (m_path.isEmpty()) { return; } - // Write our PID so the wrapper can track this exact process via kill -0. + // Open first, then verify it's a FIFO via fstat(). + // This avoids the TOCTOU race of stat() before open(). int fd = open(m_path.toUtf8().constData(), O_WRONLY | O_NONBLOCK); if (fd < 0) { qCWarning(logUpdateModal) << "FifoNotifier: cannot report PID, open failed:" << strerror(errno); return; } + struct stat st; + if (fstat(fd, &st) != 0 || !S_ISFIFO(st.st_mode)) { + qCWarning(logUpdateModal) << "FifoNotifier: path is not a FIFO, cannot report PID"; + close(fd); + return; + } + // Write our PID so the wrapper can track this exact process via kill -0. QByteArray msg = "PID:" + QByteArray::number(getpid()) + "\n"; bool ok = writeAllFifo(fd, msg, 5000); close(fd); @@ -108,6 +117,8 @@ class FifoNotifier return; } qCInfo(logUpdateModal) << "FifoNotifier: notifying FIFO" << m_path; + // Open first, then verify it's a FIFO via fstat(). + // This avoids the TOCTOU race of stat() before open(). // O_NONBLOCK so we don't hang if the reader already closed; // the script opens fd 3<> before launching us so a reader exists. int fd = open(m_path.toUtf8().constData(), O_WRONLY | O_NONBLOCK); @@ -115,6 +126,12 @@ class FifoNotifier qCWarning(logUpdateModal) << "FifoNotifier: cannot open" << m_path << ":" << strerror(errno); return; } + struct stat st; + if (fstat(fd, &st) != 0 || !S_ISFIFO(st.st_mode)) { + qCWarning(logUpdateModal) << "FifoNotifier: path is not a FIFO, cannot notify"; + close(fd); + return; + } // Write "EXIT\n" to signal exit to the wrapper. QByteArray msg("EXIT\n"); bool ok = writeAllFifo(fd, msg, 2000); diff --git a/src/dde-update/securityloaderhelper.cpp b/src/dde-update/securityloaderhelper.cpp index 0ba00e86..8b9ff90d 100644 --- a/src/dde-update/securityloaderhelper.cpp +++ b/src/dde-update/securityloaderhelper.cpp @@ -18,6 +18,7 @@ #include #include #include +#include static const QString DEFAULT_AUTH_RESOURCE = ":/misc/org.deepin.dde-update.json"; // Upper bound for the authorization response to guard against a misbehaving @@ -65,6 +66,14 @@ LoaderHandshakeInfo SecurityLoaderHelper::parseLoaderArgs(int argc, char *argv[] continue; } info.fd1 = static_cast(fd); + // Verify the fd is actually a FIFO to prevent writing + // authorization data to arbitrary open files. + struct stat st; + if (fstat(info.fd1, &st) != 0 || !S_ISFIFO(st.st_mode)) { + qWarning() << "fd1 is not a FIFO, ignoring"; + info.fd1 = -1; + continue; + } info.isLoadedByLoader = true; } else if (strcmp(argv[i], "--fd2") == 0 && i + 1 < argc) { const char *val = argv[++i]; @@ -77,6 +86,14 @@ LoaderHandshakeInfo SecurityLoaderHelper::parseLoaderArgs(int argc, char *argv[] continue; } info.fd2 = static_cast(fd); + // Verify the fd is actually a FIFO to prevent writing + // authorization data to arbitrary open files. + struct stat st; + if (fstat(info.fd2, &st) != 0 || !S_ISFIFO(st.st_mode)) { + qWarning() << "fd2 is not a FIFO, ignoring"; + info.fd2 = -1; + continue; + } } }