Repository navigation
Conversation
Log: add absolute path and executability validation for PreUninstall script Influence: fix CWE-78 command injection risk from .desktop file X-Deepin-PreUninstall field
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fly602 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 |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds pre-execution validation for the X-Deepin-PreUninstall script path, requiring an absolute, existing, executable file and aborting with warnings otherwise, mitigating command-injection and path-hijacking risks from crafted .desktop files. Sequence diagram for validated pre-uninstall script executionsequenceDiagram
participant DesktopFile
participant AppWiz
participant QFileInfo
participant Process
DesktopFile->>AppWiz: legacyRequestUninstall(desktopFileFullPath)
AppWiz->>AppWiz: Read X-Deepin-PreUninstall
AppWiz->>QFileInfo: isAbsolute()
alt path is not absolute
AppWiz-->>AppWiz: qCWarning and return
else absolute path
AppWiz->>QFileInfo: exists()
AppWiz->>QFileInfo: isExecutable()
alt missing or not executable
AppWiz-->>AppWiz: qCWarning and return
else valid executable
AppWiz->>Process: start(args[0], args.mid(1))
end
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价: 语法正确,逻辑清晰 ✅ 通过 潜在问题: 建议: [] 2. 代码质量 ✅评价: 代码结构清晰,注释完整 ✅ 通过 潜在问题: 建议: [] 3. 代码性能 ✅评价: 性能良好,资源使用合理 ✅ 通过 潜在问题: 建议: [] 4. 代码安全 🔒评价: 存在0个安全漏洞 ✅ 通过
安全漏洞详情: 建议: ['建议1: 可考虑额外检查 scriptInfo.isSymLink() 并拒绝符号链接,以消除 TOCTOU 竞态条件(OCR 建议,防御深度加固)', '建议2: 可考虑使用 QProcess::start(program, args) 形式而非字符串形式,进一步避免 QProcess::splitCommand 的潜在拆分风险(OCR 建议,防御深度加固)'] 💡 改进建议代码示例// 防御深度加固版本(可选):
QFileInfo scriptInfo(args[0]);
if (!scriptInfo.isAbsolute()) {
qCWarning(logDdeIntegration) << "Pre-uninstall script path is not absolute, aborting:" << args[0];
return;
}
// 可选:拒绝符号链接以防 TOCTOU 竞态
if (scriptInfo.isSymLink()) {
qCWarning(logDdeIntegration) << "Pre-uninstall script path is a symlink, aborting:" << args[0];
return;
}
if (!scriptInfo.exists() || !scriptInfo.isExecutable()) {
qCWarning(logDdeIntegration) << "Pre-uninstall script does not exist or is not executable, aborting:" << args[0];
return;
}
if (args.size() == 1) {
process.start(args[0]);
} else {
process.start(args[0], args.mid(1));
}本报告由 AI 代码审查工具自动生成 |
修复内容
CWE-78 命令注入风险
对
appwiz.cpp中从 .desktop 文件X-Deepin-PreUninstall字段读取的预卸载脚本增加执行前校验:新增校验
原代码
直接从 .desktop 文件读取
X-Deepin-PreUninstall字段并执行,恶意/被篡改应用包可通过该字段注入任意命令。修复后
Log: add absolute path and executability validation for PreUninstall script
Influence: fix CWE-78 command injection risk from .desktop file X-Deepin-PreUninstall field
Summary by Sourcery
Bug Fixes: