Skip to content

fix: add path traversal check in mkTempDir function#191

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
dengzhongyuan365-dev:master
Feb 3, 2026
Merged

fix: add path traversal check in mkTempDir function#191
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
dengzhongyuan365-dev:master

Conversation

@dengzhongyuan365-dev
Copy link
Contributor

@dengzhongyuan365-dev dengzhongyuan365-dev commented Feb 2, 2026

  • Implemented a validation check in the mkTempDir function to reject infix values containing "..", enhancing security against path traversal vulnerabilities.

This change improves the robustness of the temporary directory creation process.

Summary by Sourcery

Bug Fixes:

  • Reject mkTempDir infix values containing ".." to avoid potential path traversal when creating temporary directories.

- Implemented a validation check in the mkTempDir function to reject infix values containing "..", enhancing security against path traversal vulnerabilities.

This change improves the robustness of the temporary directory creation process.
@sourcery-ai
Copy link

sourcery-ai bot commented Feb 2, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adds a security-focused validation to Utils::mkTempDir to reject potentially unsafe infix values that contain path traversal sequences before constructing the temporary directory path.

File-Level Changes

Change Details Files
Add path traversal validation in temporary directory creation helper
  • Log the requested infix value at debug level before validation
  • Introduce a guard that checks whether the infix string contains ".." and treats it as invalid
  • Emit a warning log when an invalid infix is detected and abort temp directory creation by returning an empty QString
  • Leave the subsequent template construction logic unchanged, so it only runs for validated infix values
basestruct/utils.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

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @dengzhongyuan365-dev, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码的修改旨在增加安全性,防止路径遍历攻击。以下是对该代码变更的详细审查意见,包括语法逻辑、代码质量、代码性能和代码安全方面的改进建议:

1. 语法逻辑

  • 当前实现:检查 infix 是否包含 ".." 字符串。
  • 逻辑问题:仅检查 ".." 是不够的。虽然 ".." 是最常见的路径遍历序列,但攻击者可能使用其他方式绕过检查,例如:
    • 使用 URL 编码(如 %2e%2e
    • 使用 Unicode 变体(如 ..
    • 使用符号链接(symlink)
  • 改进建议
    • 使用更严格的路径规范化检查,例如 QDir::cleanPath()QFileInfo::canonicalFilePath() 来解析路径并检查是否仍在预期目录内。
    • 或者,仅允许 infix 包含安全的字符(如字母、数字、下划线和连字符),拒绝其他所有字符。

2. 代码质量

  • 当前实现:直接返回 QString(),调用者需要检查返回值是否为空。
  • 问题
    • 调用者可能忘记检查返回值,导致后续操作失败。
    • 错误处理不够明确,调用者无法区分是因路径遍历检查失败还是其他原因(如权限不足)导致的失败。
  • 改进建议
    • 使用枚举或错误码明确失败原因,例如:
      enum class TempDirError {
          Success,
          PathTraversal,
          PermissionDenied,
          Other
      };
    • 或者抛出异常(如果项目允许使用异常)。

3. 代码性能

  • 当前实现:使用 QString::contains() 检查 ".."。
  • 性能QString::contains() 的时间复杂度是 O(n),其中 n 是 infix 的长度。对于短字符串(如文件名或目录名),性能影响可以忽略不计。
  • 改进建议
    • 如果 infix 的长度可能很大(例如用户输入),可以限制其最大长度。
    • 如果性能是关键,可以使用正则表达式预编译(但可能不必要)。

4. 代码安全

  • 当前实现:检查 ".." 是一个基本的安全措施,但不够全面。
  • 潜在问题
    • 如果 infix 包含绝对路径(如 /tmp/evil),仍然可能导致安全问题。
    • 如果 infix 包含空字符(\0),可能导致截断攻击。
  • 改进建议
    • 检查 infix 是否以 / 开头(拒绝绝对路径)。
    • 检查 infix 是否包含空字符。
    • 使用 QDir::cleanPath()QFileInfo::canonicalFilePath() 来规范化路径。
    • 限制 infix 的字符集,例如仅允许 [a-zA-Z0-9_-]

改进后的代码示例

QString Utils::mkTempDir(const QString &infix)
{
    qDebug() << "Utils::mkTempDir - Creating temp dir with infix:" << infix;

    // 检查空输入
    if (infix.isEmpty()) {
        qWarning() << "Utils::mkTempDir - Empty infix";
        return QString();
    }

    // 检查路径遍历和绝对路径
    if (infix.contains("..") || infix.startsWith('/') || infix.contains('\0')) {
        qWarning() << "Utils::mkTempDir - Invalid infix contains path traversal or absolute path:" << infix;
        return QString();
    }

    // 检查字符集(可选)
    QRegularExpression safeChars("^[a-zA-Z0-9_-]+$");
    if (!safeChars.match(infix).hasMatch()) {
        qWarning() << "Utils::mkTempDir - Invalid infix contains unsafe characters:" << infix;
        return QString();
    }

    // Construct template like "/var/tmp/diskmanager-XXXXXX" or "/var/tmp/diskmanager-INFIX-XXXXXX"
    QString dirTemplate = "/var/tmp/";
    // ... 其余代码
}

总结

  • 优点:当前的修改增加了基本的安全检查,防止简单的路径遍历攻击。
  • 缺点:安全检查不够全面,可能被绕过。
  • 改进方向:增强路径规范化检查、限制字符集、明确错误处理。

通过以上改进,可以显著提高代码的安全性和健壮性。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengzhongyuan365-dev, 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:

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

@dengzhongyuan365-dev
Copy link
Contributor Author

/forcemerge

@deepin-bot deepin-bot bot merged commit c8bd559 into linuxdeepin:master Feb 3, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants