fix: Improve error handling in DMDbusHandler and DiskManagerService

[dengzhongyuan365-dev]

• Added checks for the existence of current device paths in DMDbusHandler methods to prevent crashes and log warnings when paths are not found.
• Updated DiskManagerService to handle invalid invoker UID retrieval gracefully, ensuring proper authorization checks and logging warnings for failed operations.
• Refactored Btrfs size parsing to handle out-of-bounds access more safely.

These changes enhance the robustness and reliability of device management operations.

task: https://pms.uniontech.com/task-view-386519.html

[sourcery-ai]

Sorry @dengzhongyuan365-dev, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[deepin-ci-robot]

deepin pr auto review

这份代码修改主要涉及DBus权限检查、文件系统解析和系统关键配置文件的写入。整体来看，代码质量有显著提升，特别是在安全性和健壮性方面。以下是对这三个文件修改的详细审查意见：

1. diskmanagerservice.cpp (DBus 权限与 UID 获取)

修改内容：
将直接调用 .value() 的方式改为先检查 QDBusReply 的有效性，并增加了详细的错误日志。

审查意见：

• 代码逻辑与健壮性 (优)：

  • 原代码直接调用 .value() 存在风险。如果 D-Bus 调用失败（例如服务端崩溃或连接中断），value() 可能返回未定义行为或默认构造值（如 0），这可能导致安全漏洞（将未授权用户误判为 root）。
  • 新代码增加了 isValid() 检查，并在无效时拒绝操作，这是非常正确的做法。

• 代码性能 (中)：

  • 性能影响微乎其微。虽然增加了一次 isValid() 判断，但这属于必要的系统调用开销，无法避免。

• 代码安全 (优)：

  • 显著提升。在 mount 函数中，如果无法获取调用者 UID，直接拒绝挂载是符合安全原则的“失败安全”策略。
  • 在 checkAuthorization 函数中，逻辑拆分更清晰。原代码中 serviceUid(...).value() == 0 如果失败返回 0，可能会错误地授予 root 权限。新代码修复了这个潜在的逻辑漏洞。

• 改进建议：

  • 在 mount 函数中，建议在 qWarning 中打印具体的错误信息 uidReply.error().message()，以便于排查 D-Bus 通信问题。
  • 在 checkAuthorization 中，serviceName == m_frontEndDBusName 的判断逻辑保留得很好，这通常用于允许前端服务直接调用。建议确认 m_frontEndDBusName 本身是否在初始化时经过了严格的验证，防止被伪造。

2. filesystems/btrfs.cpp (字符串解析)

修改内容：
简化了字符串索引越界的检查逻辑，修复了潜在的未初始化变量警告。

审查意见：

• 代码逻辑 (优)：

  • 原代码逻辑混乱，存在死代码。if (pos < str.length()) 块内声明了 unit 但未使用，随后又在块外无条件使用 str.at(pos)，这在 pos 等于长度时会导致越界崩溃。
  • 新代码使用三元运算符简洁地处理了边界情况，当 pos 越界时将 unit 设为 \0。

• 代码质量 (优)：

  • 代码更加简洁易读，消除了冗余逻辑。

• 代码安全 (优)：

  • 修复了潜在的数组越界访问风险。

• 改进建议：

  • 当 unit 为 \0 时，switch 语句没有 default 分支，mult 变量将保持未初始化状态（取决于编译器实现，可能包含垃圾值）。虽然原代码也是如此，但建议在 switch 后增加对 mult 的检查，或者设置默认值：

    Byte_Value mult = BYTE; // 设置默认值
    switch (unit) { ... }

3. luksoperator/luksoperator.cpp (文件写入)

修改内容：
将直接使用 QFile 覆盖写入 /etc/crypttab 改为使用 QSaveFile 进行原子写入，并增加了权限恢复逻辑。

审查意见：

• 代码逻辑 (优)：

  • 引入 QSaveFile 是极佳的改进。QSaveFile 会先写入临时文件，确保写入成功后再调用 rename(2) 原子性地替换目标文件。这避免了在写入过程中如果程序崩溃或断电导致 /etc/crypttab 内容丢失或损坏（只剩下半截数据）的风险。

• 代码安全 (优)：

  • 显著提升。修复了 TOCTOU（Time-of-check to Time-of-use）类的竞态条件风险，以及文件损坏风险。
  • 保存并恢复 origPerm 也是非常必要的细节。QSaveFile 创建临时文件时通常使用默认 umask，如果不恢复原权限，可能导致 /etc/crypttab 权限过宽（例如变成 644），虽然 crypttab 内容通常不包含极敏感密码，但保持严格的系统文件权限（如 600 或 640）是良好的安全实践。

• 代码质量 (优)：

  • 注释清晰，解释了为什么要这样做（原子写入、避免 TOCTOU）。

• 改进建议：

  • 潜在的权限竞争风险：代码逻辑是 读取旧文件权限 -> 写入新文件 -> 恢复权限。在多线程或并发环境下，如果在这期间有其他进程修改了 /etc/crypttab 的权限，setPermissions 会覆盖掉其他进程的修改。不过在磁盘管理服务中，此函数通常串行调用，风险较低。
  • 错误处理细化：QFile::setPermissions 也有可能失败（例如磁盘只读），建议检查其返回值：

    if (!QFile::setPermissions(...)) {
        qWarning() << "Failed to restore permissions for /etc/crypttab";
        // 根据业务逻辑决定是否返回 false
    }

  • 拼写错误：函数名 wirteCrypttab 拼写有误（应为 write），虽然不影响功能，但建议修正以保持代码规范。

总结

这份 Diff 修复了多处潜在的安全隐患和逻辑漏洞，代码质量很高。特别是引入 QSaveFile 和修复 D-Bus UID 检查逻辑，体现了对系统稳定性和安全性的深入考虑。

建议合并，但建议在合并前处理以下小细节：

1. btrfs.cpp 中 mult 变量的默认初始化。
2. luksoperator.cpp 中 setPermissions 的返回值检查。
3. 修正 wirteCrypttab 的拼写。

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengzhongyuan365-dev, lzwind

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[dengzhongyuan365-dev]

/merge