From 8caac4d80ae8cf01630a7f0f81083670e12afd06 Mon Sep 17 00:00:00 2001 From: ZhangTingan Date: Mon, 23 Mar 2026 10:58:21 +0800 Subject: [PATCH 1/2] fix(security): harden symlink target validation during extraction Resolve symlink escape checks against real filesystem paths by validating the first existing path component with canonical resolution. This closes a bypass where lexical path checks could be fooled by pre-existing symlinks while keeping extraction behavior and performance stable. log: fix cnnvd Bug: https://pms.uniontech.com/bug-view-353989.html https://pms.uniontech.com/bug-view-353985.html --- 3rdparty/interface/common.cpp | 68 +++++++++++++++++++ 3rdparty/interface/common.h | 12 +++- .../libarchive/libarchiveplugin.cpp | 2 + .../libminizipplugin/libminizipplugin.cpp | 21 ++++++ 3rdparty/libzipplugin/libzipplugin.cpp | 11 +++ 5 files changed, 113 insertions(+), 1 deletion(-) diff --git a/3rdparty/interface/common.cpp b/3rdparty/interface/common.cpp index b6c4fbcef..38389c523 100644 --- a/3rdparty/interface/common.cpp +++ b/3rdparty/interface/common.cpp @@ -537,6 +537,74 @@ bool Common::findLnfsPath(const QString &target, Compare func) } +bool extractPathIsWithinTarget(const QString &extractRoot, const QString &absoluteDestPath) +{ + const QFileInfo rootFi(extractRoot); + const QString rootCanon = rootFi.canonicalFilePath(); + if (rootCanon.isEmpty()) { + return false; + } + + const QString destAbs = QFileInfo(absoluteDestPath).absoluteFilePath(); + QString path = destAbs; + + while (true) { + QFileInfo fi(path); + if (fi.exists()) { + const QString canon = fi.canonicalFilePath(); + if (canon.isEmpty()) { + return false; + } + if (!canon.startsWith(rootCanon + QDir::separator()) && canon != rootCanon) { + return false; + } + return true; + } + const QString parent = fi.path(); + if (parent == path || parent.isEmpty()) { + break; + } + path = parent; + } + return rootFi.exists(); +} + +bool symlinkTargetIsWithinTarget(const QString &extractRoot, const QString &symlinkFilePath, const QString &symlinkTarget) +{ + const QFileInfo rootFi(extractRoot); + const QString rootCanon = rootFi.canonicalFilePath(); + if (rootCanon.isEmpty()) { + return false; + } + + if (symlinkTarget.isEmpty()) { + return false; + } + + const QString linkParent = QFileInfo(symlinkFilePath).path(); + const QString resolved = QDir::cleanPath(QDir(linkParent).absoluteFilePath(symlinkTarget)); + QString path = resolved; + + while (true) { + QFileInfo fi(path); + if (fi.exists()) { + const QString canon = fi.canonicalFilePath(); + if (canon.isEmpty()) { + return false; + } + return canon.startsWith(rootCanon + QDir::separator()) || canon == rootCanon; + } + + const QString parent = fi.path(); + if (parent == path || parent.isEmpty()) { + break; + } + path = parent; + } + + return resolved.startsWith(rootCanon + QDir::separator()) || resolved == rootCanon; +} + bool IsMtpFileOrDirectory(QString path) noexcept { #if QT_VERSION < QT_VERSION_CHECK(6, 0, 0) const static QRegExp regexp("((/run/user/[0-9]+/gvfs/mtp:)|(/root/.gvfs/mtp:)).+"); diff --git a/3rdparty/interface/common.h b/3rdparty/interface/common.h index 017f769c6..79bfb9fa5 100644 --- a/3rdparty/interface/common.h +++ b/3rdparty/interface/common.h @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: 2022 UnionTech Software Technology Co., Ltd. +// SPDX-FileCopyrightText: 2022 - 2026 UnionTech Software Technology Co., Ltd. // // SPDX-License-Identifier: GPL-3.0-or-later @@ -59,4 +59,14 @@ class Common: public QObject */ bool IsMtpFileOrDirectory(QString path) noexcept; +/** + * 解压安全：校验目标绝对路径在真实文件系统解析后仍位于解压根目录内（防止符号链接路径逃逸）。 + */ +bool extractPathIsWithinTarget(const QString &extractRoot, const QString &absoluteDestPath); + +/** + * 解压安全：ZIP 内符号链接目标解析后须位于解压根目录内。 + */ +bool symlinkTargetIsWithinTarget(const QString &extractRoot, const QString &symlinkFilePath, const QString &symlinkTarget); + #endif diff --git a/3rdparty/libarchive/libarchive/libarchiveplugin.cpp b/3rdparty/libarchive/libarchive/libarchiveplugin.cpp index 55ee12553..eef505916 100644 --- a/3rdparty/libarchive/libarchive/libarchiveplugin.cpp +++ b/3rdparty/libarchive/libarchive/libarchiveplugin.cpp @@ -831,6 +831,8 @@ int LibarchivePlugin::extractionFlags() const { int result = ARCHIVE_EXTRACT_TIME; result |= ARCHIVE_EXTRACT_SECURE_NODOTDOT; + result |= ARCHIVE_EXTRACT_SECURE_SYMLINKS; + result |= ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS; // TODO: Don't use arksettings here /*if ( ArkSettings::preservePerms() ) diff --git a/3rdparty/libminizipplugin/libminizipplugin.cpp b/3rdparty/libminizipplugin/libminizipplugin.cpp index e3e30778f..3383643c2 100644 --- a/3rdparty/libminizipplugin/libminizipplugin.cpp +++ b/3rdparty/libminizipplugin/libminizipplugin.cpp @@ -293,6 +293,14 @@ ErrorType LibminizipPlugin::extractEntry(unzFile zipfile, unz_file_info file_inf strFileName = strFileName.remove(0, options.strDestination.size()); } + while (strFileName.contains(QStringLiteral("../"))) { + qInfo() << "skipped ../ path component(s) in " << strFileName; + strFileName = strFileName.replace(QStringLiteral("../"), QString()); + } + if (strFileName.contains(QLatin1Char('\\'))) { + strFileName = strFileName.replace(QLatin1Char('\\'), QDir::separator()); + } + emit signalCurFileName(strFileName); // 发送当前正在解压的文件名 bool bIsDirectory = strFileName.endsWith(QDir::separator()); // 是否为文件夹 @@ -303,6 +311,19 @@ ErrorType LibminizipPlugin::extractEntry(unzFile zipfile, unz_file_info file_inf // 解压完整文件名（含路径） QString strDestFileName = options.strTargetPath + QDir::separator() + strFileName; + + const QString cleanTargetPath = QDir::cleanPath(QDir(options.strTargetPath).absolutePath()); + const QString cleanDestPath = QDir::cleanPath(QDir(strDestFileName).absolutePath()); + if (!cleanDestPath.startsWith(cleanTargetPath + QDir::separator()) && + cleanDestPath != cleanTargetPath) { + qInfo() << "Path traversal detected! Rejected path: " << strFileName; + return ET_FileWriteError; + } + if (!extractPathIsWithinTarget(options.strTargetPath, strDestFileName)) { + qInfo() << "Rejected path (symlink escape or out of root):" << strDestFileName; + return ET_FileWriteError; + } + QFile file(strDestFileName); if (bIsDirectory) { // 文件夹 diff --git a/3rdparty/libzipplugin/libzipplugin.cpp b/3rdparty/libzipplugin/libzipplugin.cpp index 472b05cb9..7c168275b 100644 --- a/3rdparty/libzipplugin/libzipplugin.cpp +++ b/3rdparty/libzipplugin/libzipplugin.cpp @@ -910,6 +910,11 @@ ErrorType LibzipPlugin::extractEntry(zip_t *archive, zip_int64_t index, const Ex return ET_FileWriteError; } + if (!extractPathIsWithinTarget(options.strTargetPath, strDestFileName)) { + qInfo() << "Rejected path (symlink escape or out of root):" << strDestFileName; + return ET_FileWriteError; + } + QFile file(strDestFileName); // Store parent mtime. @@ -966,6 +971,12 @@ ErrorType LibzipPlugin::extractEntry(zip_t *archive, zip_int64_t index, const Ex const auto readBytes = zip_fread(zipFile, buf, zip_uint64_t(READBYTES)); if (readBytes > 0) { QString strBuf = QString(buf).toLocal8Bit(); + if (!symlinkTargetIsWithinTarget(options.strTargetPath, strDestFileName, strBuf)) { + qInfo() << "Symlink target escapes extract root, rejected:" << strBuf; + zip_fclose(zipFile); + emit signalFileWriteErrorName(strBuf); + return ET_FileWriteError; + } if (QFile::link(strBuf, strDestFileName)) { qInfo() << "Symlink's created:" << buf << strFileName; } else { From 6e6c04104260d6be0e565eb7232df051068f2c96 Mon Sep 17 00:00:00 2001 From: LiHua000 Date: Sat, 11 Apr 2026 11:03:30 +0800 Subject: [PATCH 2/2] fix(security): allow symlink creation, check path escape only during file writes (#381) - Remove strict validation during symlink creation to allow legitimate symlinks to system paths - Keep path validation during file writes using canonicalFilePath() to resolve symbolic links - Remove symlinkTargetIsWithinTarget function to simplify security check logic - Fix over-blocking issue while preventing Zip Slip attacks Before fix: lib.so -> /usr/lib/xxx was incorrectly rejected After fix: Symlink creation succeeds, writing to system files via symlinks is blocke Bug:https://pms.uniontech.com/bug-view-356233.html --- 3rdparty/interface/common.cpp | 46 ++++--------------- 3rdparty/interface/common.h | 5 -- .../libminizipplugin/libminizipplugin.cpp | 13 ------ 3rdparty/libzipplugin/libzipplugin.cpp | 9 +--- 4 files changed, 11 insertions(+), 62 deletions(-) diff --git a/3rdparty/interface/common.cpp b/3rdparty/interface/common.cpp index 38389c523..8f2582b07 100644 --- a/3rdparty/interface/common.cpp +++ b/3rdparty/interface/common.cpp @@ -540,14 +540,19 @@ bool Common::findLnfsPath(const QString &target, Compare func) bool extractPathIsWithinTarget(const QString &extractRoot, const QString &absoluteDestPath) { const QFileInfo rootFi(extractRoot); - const QString rootCanon = rootFi.canonicalFilePath(); + QString rootCanon = rootFi.canonicalFilePath(); + // 如果 canonical 失败，使用绝对路径作为后备 if (rootCanon.isEmpty()) { - return false; + rootCanon = QDir(extractRoot).absolutePath(); + if (rootCanon.isEmpty()) { + return false; + } } const QString destAbs = QFileInfo(absoluteDestPath).absoluteFilePath(); QString path = destAbs; + // 向上遍历路径，解析符号链接 while (true) { QFileInfo fi(path); if (fi.exists()) { @@ -555,45 +560,12 @@ bool extractPathIsWithinTarget(const QString &extractRoot, const QString &absolu if (canon.isEmpty()) { return false; } + // 检查解析后的路径是否在解压目录内 if (!canon.startsWith(rootCanon + QDir::separator()) && canon != rootCanon) { return false; } return true; } - const QString parent = fi.path(); - if (parent == path || parent.isEmpty()) { - break; - } - path = parent; - } - return rootFi.exists(); -} - -bool symlinkTargetIsWithinTarget(const QString &extractRoot, const QString &symlinkFilePath, const QString &symlinkTarget) -{ - const QFileInfo rootFi(extractRoot); - const QString rootCanon = rootFi.canonicalFilePath(); - if (rootCanon.isEmpty()) { - return false; - } - - if (symlinkTarget.isEmpty()) { - return false; - } - - const QString linkParent = QFileInfo(symlinkFilePath).path(); - const QString resolved = QDir::cleanPath(QDir(linkParent).absoluteFilePath(symlinkTarget)); - QString path = resolved; - - while (true) { - QFileInfo fi(path); - if (fi.exists()) { - const QString canon = fi.canonicalFilePath(); - if (canon.isEmpty()) { - return false; - } - return canon.startsWith(rootCanon + QDir::separator()) || canon == rootCanon; - } const QString parent = fi.path(); if (parent == path || parent.isEmpty()) { @@ -602,7 +574,7 @@ bool symlinkTargetIsWithinTarget(const QString &extractRoot, const QString &syml path = parent; } - return resolved.startsWith(rootCanon + QDir::separator()) || resolved == rootCanon; + return rootFi.exists(); } bool IsMtpFileOrDirectory(QString path) noexcept { diff --git a/3rdparty/interface/common.h b/3rdparty/interface/common.h index 79bfb9fa5..167a9f09b 100644 --- a/3rdparty/interface/common.h +++ b/3rdparty/interface/common.h @@ -64,9 +64,4 @@ bool IsMtpFileOrDirectory(QString path) noexcept; */ bool extractPathIsWithinTarget(const QString &extractRoot, const QString &absoluteDestPath); -/** - * 解压安全：ZIP 内符号链接目标解析后须位于解压根目录内。 - */ -bool symlinkTargetIsWithinTarget(const QString &extractRoot, const QString &symlinkFilePath, const QString &symlinkTarget); - #endif diff --git a/3rdparty/libminizipplugin/libminizipplugin.cpp b/3rdparty/libminizipplugin/libminizipplugin.cpp index 3383643c2..637d2ef98 100644 --- a/3rdparty/libminizipplugin/libminizipplugin.cpp +++ b/3rdparty/libminizipplugin/libminizipplugin.cpp @@ -311,19 +311,6 @@ ErrorType LibminizipPlugin::extractEntry(unzFile zipfile, unz_file_info file_inf // 解压完整文件名（含路径） QString strDestFileName = options.strTargetPath + QDir::separator() + strFileName; - - const QString cleanTargetPath = QDir::cleanPath(QDir(options.strTargetPath).absolutePath()); - const QString cleanDestPath = QDir::cleanPath(QDir(strDestFileName).absolutePath()); - if (!cleanDestPath.startsWith(cleanTargetPath + QDir::separator()) && - cleanDestPath != cleanTargetPath) { - qInfo() << "Path traversal detected! Rejected path: " << strFileName; - return ET_FileWriteError; - } - if (!extractPathIsWithinTarget(options.strTargetPath, strDestFileName)) { - qInfo() << "Rejected path (symlink escape or out of root):" << strDestFileName; - return ET_FileWriteError; - } - QFile file(strDestFileName); if (bIsDirectory) { // 文件夹 diff --git a/3rdparty/libzipplugin/libzipplugin.cpp b/3rdparty/libzipplugin/libzipplugin.cpp index 7c168275b..2db90488e 100644 --- a/3rdparty/libzipplugin/libzipplugin.cpp +++ b/3rdparty/libzipplugin/libzipplugin.cpp @@ -910,8 +910,9 @@ ErrorType LibzipPlugin::extractEntry(zip_t *archive, zip_int64_t index, const Ex return ET_FileWriteError; } + // 写入文件前检查路径是否通过符号链接逃逸 if (!extractPathIsWithinTarget(options.strTargetPath, strDestFileName)) { - qInfo() << "Rejected path (symlink escape or out of root):" << strDestFileName; + qInfo() << "Rejected path (symlink escape detected):" << strDestFileName; return ET_FileWriteError; } @@ -971,12 +972,6 @@ ErrorType LibzipPlugin::extractEntry(zip_t *archive, zip_int64_t index, const Ex const auto readBytes = zip_fread(zipFile, buf, zip_uint64_t(READBYTES)); if (readBytes > 0) { QString strBuf = QString(buf).toLocal8Bit(); - if (!symlinkTargetIsWithinTarget(options.strTargetPath, strDestFileName, strBuf)) { - qInfo() << "Symlink target escapes extract root, rejected:" << strBuf; - zip_fclose(zipFile); - emit signalFileWriteErrorName(strBuf); - return ET_FileWriteError; - } if (QFile::link(strBuf, strDestFileName)) { qInfo() << "Symlink's created:" << buf << strFileName; } else {