Skip to content

fix: restrict path traversal check to file scheme in DEnumerator buildUrl - #379

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/snipefrom
Johnson-zs:agent/bugfix/02144741-snipe
Aug 5, 2026
Merged

fix: restrict path traversal check to file scheme in DEnumerator buildUrl#379
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/snipefrom
Johnson-zs:agent/bugfix/02144741-snipe

Conversation

@Johnson-zs

@Johnson-zs Johnson-zs commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

根因分析

DEnumeratorPrivate::buildUrl() 中的路径遍历安全检查 fileNameBa.contains('\\') 对 gio trash:/// 后端使用反斜杠分隔的扁平文件名(如 \media\user\dev\.Trash-1000\files\x)返回空 QUrl(""),导致 /media 挂载点的回收站文件无法显示和清空。

  • 回归来源: commit fd494fb(PMS #367075, 2026-07-16)引入了该检查
  • 证据: 运行时日志显示第 2 个回收站文件 URL 为 QUrl(""),触发 kIsNotTrashFileError

修复方案

将路径遍历检查限制为仅对 file:/// 或无 scheme 的本地文件系统生效,trash:/// 等 gio 虚拟文件系统跳过该检查。

改动安全评估

低风险。修改仅限制检查的适用 scheme 范围,不改变 buildUrl() 签名或返回值语义。对 file:/// scheme 行为完全不变。

Summary by Sourcery

Bug Fixes:

  • Prevent legitimate gio trash:/// entries with backslash-separated names from being rejected as invalid URLs by the path traversal safeguard.

…dUrl

1. DEnumeratorPrivate::buildUrl() 对所有 scheme 的文件名做路径遍历检查,导致 gio trash:/// 后端使用反斜杠分隔的扁平文件名被误判为恶意路径并返回空 URL;
2. 将路径遍历检查限制为仅对 file:/// 或无 scheme 的本地文件系统生效,gio trash:/// 等虚拟文件系统的合法文件名不受影响;
3. 修复手动分区场景下 /media 挂载点的回收站文件无法显示和清空的问题;

Log: 修复 buildUrl 路径遍历安全检查误伤 gio trash 反斜杠文件名导致回收站无法显示和清空的问题

PMS: BUG-372733
Bug: https://pms.uniontech.com/bug-view-372733.html

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @Johnson-zs, 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
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Johnson-zs

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

@sourcery-ai

sourcery-ai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Restricts the path traversal protection in DEnumeratorPrivate::buildUrl to only apply for local file-system URLs (file:/// or no scheme), so that gio trash:/// backends using backslash-separated flat filenames are no longer incorrectly rejected.

File-Level Changes

Change Details Files
Scope the path traversal check in DEnumeratorPrivate::buildUrl to only run for local file schemes and tighten it to canonical '..' traversal patterns.
  • Create QByteArray from fileName as before but defer validation based on URL scheme.
  • Introduce a scheme check (empty or 'file') before performing any path traversal validation.
  • Replace generic slash/backslash and '.'/'..' blocking logic with explicit checks for '../', '..\', or filenames starting with '..'.
  • Skip the traversal check entirely for non-local schemes such as 'trash', allowing backslash-containing filenames used by gio trash backends.
src/dfm-io/dfm-io/denumerator.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

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:60分

■ 【总体评价】

代码修复了trash协议下合法反斜杠路径被误拦截的问题,但引入了网络协议路径遍历防护缺失的安全风险
逻辑正确且性能良好,但因存在中危安全漏洞触发强制上限扣分

■ 【详细分析】

  • 1.语法逻辑(完全正确)✓

代码准确区分了本地文件系统与其他协议,通过精确匹配 "../"、".." 和以 ".." 开头的字符串,有效解决了原代码因宽泛拦截 "/" 和 "" 导致的 trash:// 协议文件无法加载的问题,逻辑严密无误
建议:无需修改

  • 2.代码质量(优秀)✓

注释详尽,清晰解释了 gio 的 trash:/// 后端返回反斜杠扁平路径的特殊性以及为何将安全检查限制在本地文件系统,代码意图明确,可读性高
建议:无需修改

  • 3.代码性能(高效)✓

使用 url.scheme() 获取协议类型并与 QLatin1String 进行比较,避免了不必要的深拷贝,对 QByteArray 的 contains 和 startsWith 操作时间复杂度为 O(N),在文件名长度极短的情况下性能开销可忽略不计
建议:无需修改

  • 4.代码安全(存在 1 个安全漏洞)✕

漏洞对比统计:新增漏洞 1 个,减少漏洞 0 个,持平 0 个
修复逻辑过度缩小了防护范围,将路径遍历检查限制在 scheme 为空或 file 的情况下,导致 smb://、sftp:// 等网络文件系统协议完全丧失防御能力,攻击面扩大至所有网络文件系统后端

  • 安全漏洞1(【中危】):路径遍历防护范围过窄 在 DEnumeratorPrivate::buildUrl 中,修复逻辑将路径遍历检查限制在 scheme 为空或 file 的情况下,导致对 smb://、sftp:// 等网络文件系统协议完全丧失路径遍历防御能力。如果攻击者构造恶意的网络文件系统服务(如特制SMB共享),在枚举目录时返回包含 "../" 的文件名,将绕过该检查并被拼接到最终 URL 中,可能导致客户端越权访问服务端其他目录资源 ——非常重要

  • 建议:移除对 scheme 的限制判断,直接对所有协议统一实施精确的 "../"、".." 及 startsWith("..") 检查,这样既不会误杀 trash:// 协议的合法反斜杠路径,又能确保网络协议的路径遍历防御不被削弱

■ 【改进建议代码示例】

    QByteArray fileNameBa(fileName);

    // 拦截路径遍历攻击,防止恶意文件名越权
    // gio 的 trash:/// 后端对非用户主目录挂载点的回收站文件,使用反斜杠分隔的扁平路径
    // 作为 GFileInfo 的 standard::name(例如 "\media\user\dev\.Trash-1000\files\x"),
    // 这是合法的 trash 文件名而非恶意路径,故移除对单独 '/' 和 '\' 的宽泛拦截,
    // 精确匹配路径遍历序列 "../"、"..\" 或以 ".." 开头的文件名,对所有协议生效。
    if (fileNameBa.contains("../") || fileNameBa.contains("..\\") || fileNameBa.startsWith("..")) {
        return QUrl();
    }

@deepin-ci-robot

Copy link
Copy Markdown

@Johnson-zs: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
github-pr-review-ci 7160120 link true /test github-pr-review-ci

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@Johnson-zs

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Aug 5, 2026

Copy link
Copy Markdown

This pr force merged! (status: blocked)

@deepin-bot
deepin-bot Bot merged commit 496e59b into linuxdeepin:release/snipe Aug 5, 2026
18 of 20 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.

2 participants