From a27893c068bb6238dd45769c12dc6fc835601b05 Mon Sep 17 00:00:00 2001 From: ZhangTingan Date: Fri, 10 Apr 2026 16:09:39 +0800 Subject: [PATCH] fix(security): allow symlink creation, check path escape only during file writes - 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 f77e30f7..ca5c9a21 100644 --- a/3rdparty/interface/common.cpp +++ b/3rdparty/interface/common.cpp @@ -521,14 +521,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()) { @@ -536,45 +541,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()) { @@ -583,7 +555,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 d7c2f590..34d6f52d 100644 --- a/3rdparty/interface/common.h +++ b/3rdparty/interface/common.h @@ -63,9 +63,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 79f2293a..f5dd0671 100644 --- a/3rdparty/libminizipplugin/libminizipplugin.cpp +++ b/3rdparty/libminizipplugin/libminizipplugin.cpp @@ -301,19 +301,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 23f107ae..4b360db3 100644 --- a/3rdparty/libzipplugin/libzipplugin.cpp +++ b/3rdparty/libzipplugin/libzipplugin.cpp @@ -895,8 +895,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; } @@ -956,12 +957,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 {