Repository navigation
fix(security): enforce fail-closed auth and validate path inputs - #967
Merged
deepin-bot[bot] merged 2 commits intoNov 11, 2026
Conversation
- Reject D-Bus calls when resolving the sender UID fails instead of treating the default value 0 as root (authorization fail-open) - Validate debname with the component-name whitelist in delDeb to block path traversal that deleted arbitrary directories as root - Reject '..' in syspath and verify the resolved wakeup file stays under /sys to prevent overwriting arbitrary root-owned files - Always NUL-terminate the URI built by construct_uri to fix a stack out-of-bounds read (ASan: READ of size 1025) in ippAddString - Remove now-empty subdirectories in qdelDirectory so the backup directory tree is fully deleted 修复安全问题：发送方UID查询失败时拒绝调用（原实现将默认值0误判为root 导致鉴权可被竞态绕过）；delDeb增加组件名白名单校验防止路径穿越删除任意 目录；wakeup路径校验防止覆写任意root文件；construct_uri保证NUL终止修 复栈越界读；qdelDirectory清理空子目录。 Log: 修复鉴权fail-open、路径穿越删除/覆写及URI栈越界读等安全问题 Influence: 加固 deepin-devicecontrol D-Bus 接口鉴权与输入校验，正常功能不受影响。
Reviewer's GuideImplementationReject D-Bus calls when the sender UID lookup fails, so an invalid reply cannot default to UID 0 and bypass authorization.
Ensure printer URI construction NUL-terminates a full buffer, preventing downstream reads past its end.
Validate package names before deletion and remove emptied child directories so valid backup trees are fully deleted.
Reject traversal in wakeup paths and require the resolved target to remain under /sys before allowing use of that path.
TestsUpdate URI overflow tests to assert that exactly full buffers are truncated and NUL-terminated.
Stub canonical path resolution in wakeup-device tests so their mocked existing /sys files pass the new containment check.
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
对齐 PR linuxdeepin#967 安全修复后的实际行为，更新 6 个固化旧缺陷行为的用例： 1. ConstructUri_BaseOverflow_TruncatesBase / ConstructUri_ValueOverflow_StopsAtBufferEnd： construct_uri 恰好填满缓冲区时现在截去末尾 1 字符并强制 NUL 终止（ut-t25 越界读缺陷已修复），更新断言从"恰好填满不写 NUL"改为"截断并 NUL 终止"； 2. WakeupUtilsTest 4 个 updateWakeupDeviceInfo 用例：wakeupPath 新增 canonicalFilePath 兜底校验后，QFile::exists 打桩为 true 但 QFileInfo 未打桩， 假路径 canonical 解析返回空串被误判为逃逸 /sys 导致 writeWakeupFile 不再被 调用；补充 QFileInfo::canonicalFilePath 打桩模拟真实存在的 /sys 文件。 Align 6 test cases with the actual behavior after the security fixes in PR linuxdeepin#967: construct_uri now truncates one char and force-terminates with NUL when the buffer is exactly full (ut-t25 OOB-read fixed); wakeupPath now validates the canonical path stays under /sys, so the 4 updateWakeupDeviceInfo cases stub QFileInfo::canonicalFilePath to simulate a real /sys file. Log: 更新单元测试以对齐 PR linuxdeepin#967 安全修复后的正确行为 Influence: 仅测试代码，不影响功能逻辑
Contributor
Author
|
本地全量单测验证（3281 客户端用例 + 830 服务端/control 用例）发现本 PR 会导致 6 个用例失败，均为测试固化了修复前的旧（缺陷）行为，已在 bcfa5cc 更新测试对齐： control-test（2 个）
server-test / WakeupUtilsTest（4 个）
更新后本地实测：control-test 73/73、WakeupUtilsTest 49/49 全部通过。 |
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 语法正确，逻辑清晰 ✅ 通过 潜在问题： 建议： ["construct_uri() 中 buflen==0 时 else 分支会执行 buffer[-1]='\0' 越界写，建议在函数入口增加 buflen>0 前置检查，虽然当前无调用方传 0，但防御性编程更安全。"] 2. 代码质量 ✅评价： 代码结构清晰，注释完整 ✅ 通过 潜在问题： 建议： ["考虑将 syspath.contains('..') 替换为 QDir::cleanPath(syspath) 规范化后检查是否仍以 '/' 开头且不含 '..'，或移除子串检查仅依赖 canonicalFilePath() 兜底校验。"] 3. 代码性能 ✅评价： 性能良好，资源使用合理 ✅ 通过 潜在问题： 建议： [] 4. 代码安全 🔒评价： 存在0个安全漏洞 ✅ 通过
安全漏洞详情： 建议： ['建议在 construct_uri() 入口增加 buflen > 0 的断言或前置检查，彻底消除 buflen==0 时的理论越界写。'] 💡 改进建议代码示例// construct_uri 入口增加 buflen 防御性检查
void ControlInterface::construct_uri(char *buffer, size_t buflen, const char *base, const char *value)
{
if (!buffer || buflen == 0) {
return; // 防御性检查：空缓冲区或零长度直接返回
}
// ... 原有逻辑 ...
}
// wakeupPath 改进：用 QDir::cleanPath 替代 contains("..") 子串检查
bool WakeupUtils::wakeupPath(const QString &syspath, QString &wakeuppath)
{
qCDebug(appLog) << "Finding wakeup path for syspath:" << syspath;
// 规范化路径，消除冗余的 "." 和 ".."
const QString cleaned = QDir::cleanPath(syspath);
if (cleaned.contains(QLatin1String(".."))) {
qCWarning(appLog) << "Invalid syspath contains '..':" << syspath;
return false;
}
// ... 原有逻辑 ...
}本报告由 AI 代码审查工具自动生成 |
lzwind
approved these changes
Nov 11, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: add-uos, lzwind The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
Author
|
/forcemerge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
安全修复 Summary
修复 deepin-devicemanager D-Bus 接口（deepin-devicecontrol）的四个安全问题（对应内部安全报告 0007/0009）：
1. 鉴权 fail-open（根因，P0）
getUserAuthorPasswd()中serviceUid()查询失败时，QDBusReply::value()返回默认值 0（root），未认证调用被误判为 root 直接放行。攻击者用 NO_REPLY 发送调用后立即断开连接，即可在GetConnectionUnixUser返回NameHasNoOwner时绕过 polkit。修复：查询失败时 fail-closed 拒绝。
2. delDeb 路径穿越（P0）
delDeb(debname)直接拼接DB_PATH/driver/<debname>后递归删除，无组件名校验，../../../../var/lib/xxx可穿越删除任意 root 目录/文件。修复：与
backupDebFd一致使用FdUtil::isValidComponentName白名单校验。3. setWakeupMachine 路径穿越（P1）
wakeupPath()拼接 "/sys" + syspath 后写 "enabled"/"disabled"，syspath 含..时可覆写任意 root 文件前 7 字节。修复：拒绝含
..的 syspath，并校验解析后的文件路径必须位于/sys之下。4. enablePrinter 栈越界读（P1）
construct_uri()在缓冲区恰好填满时不写 NUL，ippAddString()内部strlen()越界读。999 字符 printer name 触发 ASanstack-buffer-overflow READ of size 1025@ controlinterface.cpp:185，daemon 崩溃。修复：缓冲区填满时截断并保证 NUL 终止。
附带
qdelDirectory()递归删除时清理空子目录，使备份目录树完整删除（原实现残留空目录骨架）。自测
测试环境：私有 D-Bus（未触碰系统 bus）；非 root（uid 24129）与非特权 userns root 两种身份；Release（DISABLE_POLKIT=false）与 ASan 双构建。
影响范围
仅加固 deepin-devicecontrol 的鉴权与输入校验逻辑，正常授权流程（合法 root 调用、polkit 认证）行为不变。
Summary by Sourcery
Harden deepin-devicecontrol authorization and path handling against security bypasses, arbitrary file deletion or modification, and malformed printer input.
Bug Fixes:
Enhancements:
Tests: