feat: Synchronize the V20 security vulnerability fix to V25

[LiHua000]

cherry-pick
0116e20 fix(security): allow symlink creation, check path escape only during file writes (#381)

e3f6b70 fix(security): harden symlink target validation during extraction

task: https://pms.uniontech.com/task-view-389585.html

Summary by Sourcery

Harden archive extraction security to prevent path traversal and symlink-based escapes from the extraction root.

Bug Fixes:

• Normalize and sanitize archive entry paths by removing "../" components and normalizing path separators before extraction.
• Validate destination paths against the extraction root at file-write time to block symlink traversal outside the target directory.
• Enable additional libarchive security flags to disallow unsafe symlinks and absolute paths during extraction.

Enhancements:

• Add a shared utility to verify that a resolved filesystem path remains within a specified extraction root.
• Update copyright metadata in the common header to reflect the current year range.

[sourcery-ai]

Reviewer's Guide

Synchronizes previously implemented security hardening for archive extraction into the V25 branch by tightening path validation, normalizing extracted filenames, and enabling stricter libarchive extraction flags to prevent directory traversal and symlink escape vulnerabilities.

Sequence diagram for secured path validation in LibzipPlugin::extractEntry

sequenceDiagram
    participant LibzipPlugin as LibzipPlugin::extractEntry
    participant Common as extractPathIsWithinTarget
    participant QFile as QFile

    LibzipPlugin->>LibzipPlugin: build strDestFileName
    LibzipPlugin->>Common: extractPathIsWithinTarget(options.strTargetPath, strDestFileName)
    alt [path is within target]
        Common-->>LibzipPlugin: true
        LibzipPlugin->>QFile: QFile(strDestFileName)
        LibzipPlugin->>QFile: open(QIODevice::WriteOnly)
        QFile-->>LibzipPlugin: write handle
        LibzipPlugin->>LibzipPlugin: write extracted data
    else [symlink escape or invalid path]
        Common-->>LibzipPlugin: false
        LibzipPlugin->>LibzipPlugin: return ET_FileWriteError
    end

Loading

File-Level Changes

Change	Details	Files
Add shared helper to validate that an extraction destination path remains within a target root after resolving symlinks.	Introduce extractPathIsWithinTarget helper in common.cpp and declare it in common.h Resolve canonical path of the extraction root with a fallback to absolute path if canonicalization fails Walk up the destination path hierarchy, resolving existing components via QFileInfo::canonicalFilePath to detect symlink escapes Ensure the resolved path starts with the extraction root or equals it; otherwise treat it as invalid	3rdparty/interface/common.cpp 3rdparty/interface/common.h
Normalize incoming zip entry names to strip traversal components and unify path separators during extraction.	Remove all occurrences of "../" from the computed destination relative path before extraction Replace any backslashes in entry names with the platform directory separator Log when "../" components are stripped to aid debugging and auditing	3rdparty/libminizipplugin/libminizipplugin.cpp
Harden zip extraction by enforcing the new path-within-root check before writing extracted files.	Invoke extractPathIsWithinTarget with the configured target path and the final destination filename prior to opening the file Reject the extraction with ET_FileWriteError and log a message when a symlink escape or invalid path is detected	3rdparty/libzipplugin/libzipplugin.cpp
Enable libarchive built-in protections against dangerous paths and symlinks during extraction.	Add ARCHIVE_EXTRACT_SECURE_SYMLINKS to disallow extraction through unsafe symlinks Add ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS to reject absolute paths inside archives	3rdparty/libarchive/libarchive/libarchiveplugin.cpp

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've left some high level feedback:

• The strFileName sanitization in LibminizipPlugin::extractEntry repeatedly replacing "../" substrings is quite ad-hoc and may mangle legitimate names (e.g. ".../file"); consider using QDir::cleanPath on a normalized, forward-slash-only path and then rejecting any cleaned path that starts with "../" or is absolute instead of textually stripping components.
• In extractPathIsWithinTarget, it would be safer to normalize both rootCanon and the candidate path (e.g. removing trailing separators, normalizing case where relevant) before the startsWith comparison to avoid edge cases around paths like /tmp/root vs /tmp/root2 or paths with trailing slashes.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The `strFileName` sanitization in `LibminizipPlugin::extractEntry` repeatedly replacing `"../"` substrings is quite ad-hoc and may mangle legitimate names (e.g. `".../file"`); consider using `QDir::cleanPath` on a normalized, forward-slash-only path and then rejecting any cleaned path that starts with `"../"` or is absolute instead of textually stripping components.
- In `extractPathIsWithinTarget`, it would be safer to normalize both `rootCanon` and the candidate path (e.g. removing trailing separators, normalizing case where relevant) before the `startsWith` comparison to avoid edge cases around paths like `/tmp/root` vs `/tmp/root2` or paths with trailing slashes.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[deepin-bot]

TAG Bot

New tag: 6.5.28
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #412

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: LiHua000, lzwind

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[deepin-ci-robot]

deepin pr auto review

这份代码的主要目的是修复解压缩时的路径遍历和符号链接逃逸漏洞，这是一个非常关键的安全修复。整体思路是正确的：在底层库层面增加安全标志，在业务代码层面增加路径校验。

但在代码逻辑、安全性和性能方面存在一些需要改进的地方。以下是详细的审查意见：

1. 语法与逻辑

问题 1.1：extractPathIsWithinTarget 中的逻辑缺陷

• 代码位置：3rdparty/interface/common.cpp 第 555 - 564 行
• 问题描述：向上遍历路径时，只要找到一个存在的父目录，并判断其在 rootCanon 内，就直接 return true;。但这忽略了目标文件本身可能是一个指向外部的符号链接。如果 absoluteDestPath 是 /target/malicious_link，而 malicious_link 指向 /etc/passwd，代码会先检查 /target/（存在且在内部），然后直接返回 true，并未校验最终文件实体的真实路径。
• 改进意见：应该先尝试获取 absoluteDestPath 自身的 canonicalFilePath()，如果它存在（即文件已被创建或链接已存在），则直接校验它；如果不存在，再向上遍历校验其父目录，确保创建路径的每一步都是安全的。

问题 1.2：libminizipplugin.cpp 中的路径清理逻辑极其脆弱

• 代码位置：3rdparty/libminizipplugin/libminizipplugin.cpp 第 296 - 301 行
• 问题描述：使用 replace("../", "") 来剔除路径遍历字符是非常不安全的。攻击者可以通过构造如 ....//、..%2F 或 ..././ 等变体来绕过此检查。例如：....// 经过 replace("../", "") 后会变成 ../，依然可以逃逸。
• 改进意见：应当使用 Qt 提供的标准路径规范化函数 QDir::cleanPath()，它会安全地解析 ..、. 和多余的斜杠。如果检测到清理前后的路径不一致，或者清理后的路径跳出了目标目录，应当直接拒绝解压，而不是简单地删除字符串。

2. 代码安全

问题 2.1：extractPathIsWithinTarget 存在 TOCTOU (Time-of-Check to Time-of-Use) 竞态条件风险

• 问题描述：该函数使用 QFileInfo::canonicalFilePath() 解析符号链接。在校验通过后、实际创建/写入文件前，攻击者如果迅速将已校验的目录替换为一个符号链接（即 /target/safe_dir/ 被换成了指向 /etc/ 的链接），依然可以实现逃逸。
• 改进意见：在 Qt 纯用户态下很难完全避免 TOCTOU，但可以通过调整校验时机来降低风险。应该在文件创建并打开之后（或在写入前一刻），对已打开的文件描述符进行校验，而不是仅仅在路径拼接阶段校验。如果必须保留此函数，建议在注释中明确标注此函数存在理论上的 TOCTOU 风险。

问题 2.2：跨平台路径分隔符处理不一致

• 问题描述：libminizipplugin.cpp 中将 \ 替换为 /，这是好的。但在 extractPathIsWithinTarget 中，你使用了 canon.startsWith(rootCanon + QDir::separator())。在 Windows 上，QDir::separator() 是 \，而 canonicalFilePath() 通常返回 /（Qt 内部统一使用 /），这会导致 startsWith 判断失败。
• 改进意见：Qt 的 canonicalFilePath() 和 absoluteFilePath() 返回的路径分隔符始终是 /，因此硬编码使用 / 更安全，或者直接比较 canon.size() > rootCanon.size() && canon.startsWith(rootCanon) && canon[rootCanon.size()] == QLatin1Char('/')。

问题 2.3：版权声明修改

• 问题描述：common.h 中将版权时间从 2022 改为 2022 - 2026。除非有明确的法务要求，通常不建议在代码提交中修改版权时间范围，这属于无关变更。

3. 代码性能

问题 3.1：QString::replace 在循环中的低效使用

• 代码位置：3rdparty/libminizipplugin/libminizipplugin.cpp 第 296 行
• 问题描述：while (strFileName.contains("../")) { strFileName = strFileName.replace("../", ""); } 每次循环都要先 contains 再 replace，导致对字符串进行多次遍历和内存重分配。
• 改进意见：如上文所述，直接使用 QDir::cleanPath() 一次搞定，性能和安全性都更好。

---

💡 改进后的代码建议

1. extractPathIsWithinTarget 改进版

bool extractPathIsWithinTarget(const QString &extractRoot, const QString &absoluteDestPath)
{
    const QFileInfo rootFi(extractRoot);
    QString rootCanon = rootFi.canonicalFilePath();
    // 如果 canonical 失败，使用绝对路径作为后备
    if (rootCanon.isEmpty()) {
        rootCanon = QDir(extractRoot).absolutePath();
        if (rootCanon.isEmpty()) {
            return false;
        }
    }
    // 统一使用 '/' 作为分隔符，避免跨平台问题
    rootCanon = QDir::cleanPath(rootCanon);

    const QFileInfo destFi(absoluteDestPath);
    
    // 1. 如果目标路径已经存在（例如是已存在的符号链接），直接校验其真实路径
    if (destFi.exists()) {
        const QString canon = QDir::cleanPath(destFi.canonicalFilePath());
        if (canon.isEmpty()) {
            return false;
        }
        // 检查是否在根目录内（等于根目录，或是根目录的子路径）
        if (canon == rootCanon || (canon.length() > rootCanon.length() && canon.startsWith(rootCanon) && canon.at(rootCanon.length()) == QLatin1Char('/'))) {
            return true;
        }
        return false;
    }

    // 2. 如果目标路径不存在，向上遍历校验其存在的父目录
    QString path = QDir::cleanPath(destFi.absoluteFilePath());
    while (true) {
        QFileInfo fi(path);
        if (fi.exists()) {
            const QString canon = QDir::cleanPath(fi.canonicalFilePath());
            if (canon.isEmpty()) {
                return false;
            }
            // 只要存在的父目录在根目录内，新建的子目录理论上也是安全的
            if (canon == rootCanon || (canon.length() > rootCanon.length() && canon.startsWith(rootCanon) && canon.at(rootCanon.length()) == QLatin1Char('/'))) {
                return true;
            }
            return false;
        }

        const QString parent = fi.path();
        if (parent == path || parent.isEmpty()) {
            break;
        }
        path = parent;
    }

    return false;
}

2. libminizipplugin.cpp 路径清理改进版

将不安全的 while + replace 替换为 Qt 的安全 API：

// 移除不安全的 while 循环和 replace
// 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());
}

// 使用 QDir::cleanPath 安全地解析路径
QString cleanFileName = QDir::cleanPath(strFileName);

// 安全检查：如果清理后的路径跳出了当前目录（例如包含了试图逃逸的 ../），直接拒绝
if (cleanFileName.startsWith(QStringLiteral("../")) || cleanFileName.contains(QStringLiteral("/../"))) {
    qWarning() << "Rejected potentially malicious path (path traversal detected):" << strFileName;
    return ET_FileWriteError; // 或者其他合适的错误码
}

// 如果路径被清理过，使用清理后的路径
if (cleanFileName != strFileName) {
    qInfo() << "Sanitized path from" << strFileName << "to" << cleanFileName;
    strFileName = cleanFileName;
}

3. libzipplugin.cpp 的补充

你在 libzipplugin.cpp 中加入了 extractPathIsWithinTarget 检查，这很好。但请注意，libminizipplugin.cpp 中并没有加入这个检查。为了保持一致性，建议在 libminizipplugin.cpp 的文件写入前也加上同样的 extractPathIsWithinTarget 检查，形成双保险（字符串清理 + 文件系统实体验证）。

[LiHua000]

/merge