Skip to content

fix(eventlog): 修复 .desktop 文件名命令注入漏洞#1113

Merged
robertkill merged 1 commit into
linuxdeepin:masterfrom
robertkill:fix/eventlog-command-injection
May 20, 2026
Merged

fix(eventlog): 修复 .desktop 文件名命令注入漏洞#1113
robertkill merged 1 commit into
linuxdeepin:masterfrom
robertkill:fix/eventlog-command-injection

Conversation

@robertkill
Copy link
Copy Markdown
Contributor

@robertkill robertkill commented May 20, 2026

Summary

session/eventlog 模块中,getPackageName 使用 exec.Command("/bin/bash", "-c", ...) 执行 dpkg -S 查询归属包。当 .desktop 文件名包含 shell 元字符(如 $(cmd))时,会被 bash 展开执行。

修复

exec.Command("/bin/bash", "-c", strings.Join(dpkg, " ")) 改为 exec.Command("dpkg", "-S", desktop),直接传递参数给系统调用,不经过 shell 解释。

受影响版本

6.1.89 及 master 分支均存在此问题。

测试

  • 创建名为 $(id).desktop 的 .desktop 文件,验证修复前会执行命令,修复后 id 命令不会被展开
  • 正常 .desktop 文件的包名查询功能不受影响

安全说明

dde-session-daemon 以用户权限运行(非 root),此漏洞不构成提权,但属于应在 code review 中消灭的代码模式。

Summary by Sourcery

Bug Fixes:

  • Prevent command injection in dpkg package lookup by invoking dpkg directly without passing through a shell when processing .desktop filenames.

… lookup

Use exec.Command("dpkg", "-S", desktop) directly instead of
exec.Command("/bin/bash", "-c", ...) to prevent shell metacharacter
expansion in the desktop file path.

A malicious .desktop file named e.g. $(cmd).desktop would have the
embedded command executed when the application is launched and eventlog
tries to look up its package name via dpkg.
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented May 20, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR fixes a command injection vulnerability in the session/eventlog module by changing how dpkg is executed to resolve .desktop file ownership, avoiding shell interpretation of potentially malicious filenames.

Sequence diagram for dpkg invocation change in appEventCollector

sequenceDiagram
    participant appEventCollector
    participant OS

    appEventCollector->>OS: exec.Command("/bin/bash", "-c", "dpkg -S desktop")
    OS-->>appEventCollector: cmd.Output()

    alt After_fix
        appEventCollector->>OS: exec.Command("dpkg", "-S", desktop)
        OS-->>appEventCollector: cmd.Output()
    end
Loading

File-Level Changes

Change Details Files
Eliminate shell-based command execution for dpkg lookup to prevent command injection via .desktop filenames.
  • Replace exec.Command invocation that used /bin/bash -c with a direct exec.Command call to dpkg
  • Stop building the dpkg command line via strings.Join and pass desktop filename as a separate argument to the exec call
  • Keep surrounding logging and package lookup flow unchanged aside from the command invocation details
session/eventlog/app_event.go

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

你好!我是CodeGeeX。我已仔细审查了你提供的 Git Diff 代码。

这次修改的核心是将通过 /bin/bash -c 执行 dpkg 命令的方式,改为了更直接的 exec.Command("dpkg", "-S", desktop) 方式。这是一个非常好的改进,但代码中仍存在一些潜在的风险和可以优化的空间。以下是我的详细审查意见:

1. 语法与逻辑 (语法正确,逻辑有改进空间)

  • 当前状态:修改后的代码 exec.Command("dpkg", "-S", desktop) 语法完全正确,逻辑上也更清晰。Go 的 os/exec 包会直接将参数传递给新进程,无需通过 shell 解释,这避免了原来 strings.Join 可能带来的参数解析问题。
  • 潜在问题:原代码使用 /bin/bash -c 时,如果 desktop 变量包含空格或特殊字符,shell 会将其拆分为多个参数或报错;而新代码中,desktop 作为独立参数传递给 dpkg -S,即使包含空格也会被当作一个完整的文件路径,这在逻辑上是更准确的。但是,这也意味着如果 desktop 路径本身存在异常(如前后带有空白符),会直接导致 dpkg 查询失败。

2. 代码质量 (建议增加输入校验与错误处理)

  • 输入校验缺失:在调用系统命令前,没有对 desktop 变量进行有效性检查。如果 desktop 为空字符串,dpkg -S "" 会产生不必要的错误输出。
  • 错误处理不足cmd.Output() 仅返回了标准输出和 error。如果 dpkg 命令执行失败(例如文件不属于任何包,或者 dpkg 未安装),err 将非空,但当前代码片段看不出后续如何处理。如果直接使用 buf,可能会在后续处理中引发意外。

3. 代码性能 (表现良好)

  • 性能提升:去掉 /bin/bash -c 意味着不再需要启动一个 Bash 子进程,然后再由 Bash 启动 dpkg。这减少了一次进程创建的开销,节省了内存和 CPU 资源,性能上是明显的提升。
  • 路径查询开销dpkg -S 本身在大型系统上可能会有些耗时(因为它需要搜索 dpkg 的数据库),但这属于业务逻辑的固有开销,此处的调用方式已经是最优解。

4. 代码安全 (显著提升,但仍有防御空间)

  • 命令注入风险已消除:这是此次修改最大的安全收益!原代码 exec.Command("/bin/bash", "-c", strings.Join(dpkg, " ")) 存在严重的命令注入漏洞。如果 desktop 变量来源于外部输入(如文件名被恶意构造为 foo; rm -rf /),Bash 会直接执行恶意命令。新代码使用了 Go 的参数数组传递,dpkg 会将 desktop 原样视为查询参数,彻底阻断了命令注入的途径。
  • 路径穿越/信息泄露风险:虽然解决了命令注入,但如果 desktop 是一个软链接且指向敏感路径,或者被构造为类似 ../../etc/passwd 的路径,dpkg -S 可能会返回意料之外的信息。不过结合上下文看,前面似乎已经处理了软链接(real path is %v),建议确保传入的 desktop 是经过清洗的绝对路径。

💡 改进建议及重构代码

结合以上分析,建议对代码进行如下加固:

  1. 校验 desktop 路径:确保其不为空且为绝对路径。
  2. 处理 dpkg 未找到的情况:在某些容器或最小化系统中,dpkg 可能不存在,应捕获 exec.ErrNotFound
  3. 区分标准输出和标准错误:当 dpkg -S 找不到包时,会向 Stderr 输出错误信息并返回 exit code 1。建议使用 cmd.CombinedOutput() 或分别捕获,避免错误信息丢失。

改进后的代码示例:

// 确保路径非空且有效(结合上文,desktop 应该已经是绝对路径)
if desktop == "" {
    logger.Warningf("desktop path is empty, skip dpkg query")
    return // 或 continue,取决于外层逻辑
}

// 直接执行命令,无需 bash -c
cmd := exec.Command("dpkg", "-S", desktop)
logger.Debugf("dpkg command is %v", cmd)

// 使用 CombinedOutput 捕获输出和错误,防止 dpkg 的错误信息打印到宿主进程的 stderr
buf, err := cmd.CombinedOutput() 
if err != nil {
    // 如果是命令不存在
    if errors.Is(err, exec.ErrNotFound) {
        logger.Warningf("dpkg command not found in system")
        return
    }
    
    // dpkg -S 找不到对应包时也会返回 err (exit status 1)
    // 此时的 buf 中包含了 dpkg 提示的 "no path found matching pattern" 等信息
    logger.Debugf("dpkg query failed for %s: %v, output: %s", desktop, err, strings.TrimSpace(string(buf)))
    return
}

// 正常处理 buf (包名信息)
packageName := strings.TrimSpace(string(buf))
logger.Debugf("found package: %s for desktop: %s", packageName, desktop)

总结:你的修改方向非常正确,成功修复了潜在的命令注入漏洞并提升了性能。在此基础上,增加边界条件检查和更完善的错误处理,将使这段代码达到生产级别的健壮性。

Copy link
Copy Markdown

@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.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fly602, robertkill

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

@robertkill robertkill merged commit 5e8114d into linuxdeepin:master May 20, 2026
16 of 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