Skip to content

fix(editor): replace MD5 with SHA-256 and fix D-Bus caller verification - #498

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/eaglefrom
tianming-1996:agent/security-fix-393519
Jul 31, 2026
Merged

fix(editor): replace MD5 with SHA-256 and fix D-Bus caller verification#498
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/eaglefrom
tianming-1996:agent/security-fix-393519

Conversation

@tianming-1996

@tianming-1996 tianming-1996 commented Jul 31, 2026

Copy link
Copy Markdown

Replace weak MD5 hash with SHA-256 in backup file naming and fix polkit authorization to use SystemBusNameSubject with D-Bus caller's system bus name instead of UnixProcessSubject with daemon's own PID. Also tighten D-Bus policy config and fix backup cleanup naming.

将备份文件名生成中的弱密码 MD5 算法替换为 SHA-256,并修复 polkit
授权使用 SystemBusNameSubject 配合 D-Bus 调用者 system bus name, 替代原先错误使用守护进程自身 PID 的 UnixProcessSubject。同时收紧
D-Bus 策略配置,修复备份清理文件名不匹配的问题。

Log: 修复弱密码算法和D-Bus调用者验证缺陷
PMS: TASK-393519
Influence: 提升密码算法合规性,消除潜在权限提升风险,符合UOS V20安全基线。

Summary by Sourcery

Strengthen backup file hashing and D-Bus/Polkit authorization to improve security and fix backup cleanup naming.

New Features:

  • Use SHA-256-based string hashing for generating backup and auto-backup file names instead of plain filenames.

Bug Fixes:

  • Fix backup cleanup logic to correctly remove backup files that use hashed base names.
  • Correct Polkit authorization to validate the D-Bus caller using its system bus name instead of the daemon process PID.
  • Tighten D-Bus policy configuration so only the editor daemon interface is exposed as intended.

Enhancements:

  • Unify string hashing into a generic SHA-256 helper and update call sites for backup handling.
  • Extend the D-Bus saveFile path to propagate the caller’s bus name through the adaptor for consistent authorization.
  • Clarify comments related to save-as behavior and hashing implementation timestamps.

@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 @tianming-1996, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@sourcery-ai

sourcery-ai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Reviewer's Guide

This PR replaces MD5-based backup filename hashing with a SHA-256 helper, realigns all backup creation/cleanup code around the new helper, and fixes D-Bus/Polkit integration to authorize based on the caller’s system bus name with tightened D-Bus policy configuration.

Sequence diagram for updated D-Bus saveFile authorization flow

sequenceDiagram
    actor DbusClient
    participant DbusAdaptor
    participant DBus
    participant PolicyKitHelper
    participant Authority

    DbusClient->>DbusAdaptor: saveFile(filepath, text, encoding)
    DbusAdaptor->>DbusAdaptor: message()
    DbusAdaptor->>DBus: saveFile(path, text, encoding, callerBusName)
    DBus->>PolicyKitHelper: checkAuthorization(com.deepin.editor.saveFile, callerBusName)
    PolicyKitHelper->>Authority: checkAuthorizationSync(actionId, SystemBusNameSubject(appBusName), AllowUserInteraction)
    Authority-->>PolicyKitHelper: Result Yes/No
    PolicyKitHelper-->>DBus: bool
    DBus-->>DbusAdaptor: bool
    DbusAdaptor-->>DbusClient: bool
Loading

File-Level Changes

Change Details Files
Replace MD5-based filename hashing with a SHA-256 helper and update all backup-related code paths to use it consistently.
  • Rename Utils::getStringMD5Hash to a generic getStringHash helper and switch implementation to SHA-256 via QCryptographicHash
  • Update backup file path generation in auto backup and manual backup flows to use the new hash helper instead of the raw base filename
  • Align backup cleanup logic so it removes files whose names include the SHA-256 hash, ensuring created and deleted backup filenames match
src/common/utils.cpp
src/common/utils.h
src/widgets/window.cpp
src/startmanager.cpp
Fix Polkit authorization to validate the actual D-Bus caller using its system bus name instead of the daemon’s PID, propagating the bus name through the D-Bus stack.
  • Extend DbusAdaptor to inherit QDBusContext and capture the caller’s system bus name via message().service()
  • Change DBus::saveFile and its adaptor to accept and pass through the caller’s bus name when invoking the backend save logic
  • Update PolicyKitHelper::checkAuthorization to take a system bus name string and use SystemBusNameSubject for authorization, including basic input validation
daemon/src/dbusadaptor.h
daemon/src/dbusadaptor.cpp
daemon/src/dbus.h
daemon/src/dbus.cpp
daemon/src/policykithelper.h
daemon/src/policykithelper.cpp
Tighten D-Bus policy configuration to restrict ownership and clarify which interfaces may be called on the daemon service.
  • Ensure only root can own the com.deepin.editor.daemon service while allowing it to send messages to itself explicitly
  • Remove default-context ability for non-root users to own the daemon service
  • Restrict allowed send_destination rules to the daemon’s main interface plus DBus introspection and properties, avoiding overly broad permissions
com.deepin.editor.conf
Minor comment and metadata updates to reflect new behavior and improve documentation.
  • Update Chinese comments in save-as logic to improve phrasing while preserving meaning
  • Annotate the hash helper comment to mention the SHA-256 migration and date
  • Prepare policy file section for potential Polkit rule adjustments (file touched but not shown in diff body)
src/widgets/window.cpp
src/common/utils.h
com.deepin.editor.policy

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

Replace weak MD5 hash with SHA-256 in backup file naming and fix
polkit authorization to use SystemBusNameSubject with D-Bus caller's
system bus name instead of UnixProcessSubject with daemon's own PID.
Also tighten D-Bus policy config and fix backup cleanup naming.

将备份文件名生成中的弱密码 MD5 算法替换为 SHA-256,并修复 polkit
授权使用 SystemBusNameSubject 配合 D-Bus 调用者 system bus name,
替代原先错误使用守护进程自身 PID 的 UnixProcessSubject。同时收紧
D-Bus 策略配置,修复备份清理文件名不匹配的问题。

Log: 修复弱密码算法和D-Bus调用者验证缺陷
PMS: TASK-393519
Influence: 提升密码算法合规性,消除潜在权限提升风险,符合UOS V20安全基线。
@tianming-1996
tianming-1996 force-pushed the agent/security-fix-393519 branch from 4d8dc9e to d8fd49c Compare July 31, 2026 08:48
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:100分

■ 【总体评价】

代码成功修复了Polkit授权绕过、D-Bus服务劫持和弱哈希算法等严重安全漏洞,整体安全性得到显著提升。
逻辑正确,代码质量良好,无性能问题且未引入新漏洞,故给予满分。

■ 【详细分析】

  • 1.语法逻辑(基本正确)✓

DbusAdaptor::saveFile 中通过继承 QDBusContext 获取 message().service(),并利用 QMetaObject::invokeMethodcallerBusName 传递给底层 DBus::saveFilePolicyKitHelper::checkAuthorization 正确接收字符串并构造 SystemBusNameSubject 进行同步校验。
潜在问题:无
建议:无

  • 2.代码质量(良好)✓

更新了 SPDX 版权年份,将 getStringMD5Hash 重命名为 getStringHash 并更新注释,使得函数名与实际使用的 SHA-256 算法相符。D-Bus 配置文件结构清晰,权限划分明确。
潜在问题:无
建议:无

  • 3.代码性能(无性能问题)✓

哈希算法由 MD5 升级为 SHA-256,虽然计算开销略有增加,但该函数仅用于文件备份路径生成,非高频调用路径,对整体性能无实质影响。
潜在问题:无
建议:无

  • 4.代码安全(存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 3 个,持平 0 个
本次变更有效修复了原有的 PID 复用竞争条件、D-Bus 服务名劫持和弱哈希算法问题,未引入新的安全漏洞。

  • 建议:继续保持对授权主体和权限配置的严格审查,确保所有特权操作均经过严格的身份验证。

■ 【改进建议代码示例】

// daemon/src/dbus.cpp
// 建议在文件保存前增加路径规范化与遍历检查,防止越权写入
bool DBus::saveFile(const QByteArray &path, const QByteArray &text, const QByteArray &encoding, const QString &callerBusName)
{
    const QString filepath = QString::fromUtf8(path);
    
    // 规范化路径并检查是否包含 ".." 防止路径遍历
    QString canonicalPath = QFileInfo(filepath).canonicalFilePath();
    if (canonicalPath.isEmpty() || filepath.contains("..")) {
        qWarning() << "Invalid file path detected:" << filepath;
        return false;
    }

    // 使用 D-Bus 调用者的 system bus name 进行 polkit 授权校验
    if (PolicyKitHelper::instance()->checkAuthorization("com.deepin.editor.saveFile", callerBusName)) {
        // Create file if filepath is not exists.
        if (!Utils::fileExists(canonicalPath)) {
            QString directory = QFileInfo(canonicalPath).dir().absolutePath();
            // ... 原有创建目录和文件逻辑 ...
        }
        // ... 原有保存文件逻辑 ...
    }
    return false;
}

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: max-lvs, tianming-1996

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

@tianming-1996

Copy link
Copy Markdown
Author

/merge

@deepin-bot
deepin-bot Bot merged commit 9e690ab into linuxdeepin:release/eagle Jul 31, 2026
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.

3 participants