mirror of
https://github.com/openharmony/ability_ability_runtime.git
synced 2026-08-24 12:43:16 -04:00
openharmony_ci
8e3272b0af
fix bug
Created-by: xhz-sz
Commit-by: xhz-sz
Merged-by: openharmony_ci
Description: **IssueNo**: https://gitcode.com/openharmony/ability_ability_runtime/issues/15939
**Description**:
**稳定性自检：**
| 自检项 | 自检结果 |
| ------------------------------------------------------------ | -------- |
| 涉及跨进程调用的相关操作需要抛至主线程或加锁防止并发 | |
| 成员变量进行赋值或创建需要排查并发 | |
| 谨慎在lambda表达式中使用引用捕获 | |
| 谨慎在未经拷贝的情况下使用外部传入的string、C字符串 | |
| map\vector\list\set等stl模板类使用时需要排查并发 | |
| 谨慎考虑加锁范围 | |
| 在IPC通信中谨慎使用同步通信方式 | |
| 禁止传递this指针至其他模块或线程（特别是eventhandler任务） | |
| 禁止将外部传入的裸指针在内部直接构造智能指针 | |
| 禁止多个独立创建的智能指针管理同一地址 | |
| 禁止在析构函数中抛异步任务 | |
| 禁止js对象在非js线程（例如在IPC线程）创建、使用或销毁 | |
| 禁止在对外接口中未经判空直接使用外部传入的指针 | |
| 禁止接口返回局部变量引用 | |
| 禁止在信号函数中加锁 | |
| 禁止在关键流程（SA启动、应用启动等主流程）执行耗时的操作 | |
| 禁止将同一个cpp编译在不同的so中 | |
**安全编码自检：**
| 自检项 | 自检结果 |
| -------------------------------------------------------------- | -------- |
| 裸指针避免通过隐式转换构造为sptr | |
| json对象在取值之前必须先判断类型，避免类型不匹配 | |
| 序列化时必须对传入的数组大小进行校验，避免出现超大数组 | |
| 避免使用未明确位宽的整型，选择使用int8_t、uint8_t等类型 | |
| 外部传入的路径要做规范化校验，对路径中的.、..、../等特殊字符严格校验 | |
| 指针变量、表示资源描述符的变量、bool变量必须赋初值 | |
| readParcelable获取的对象使用前需要判空 | |
| 分配和释放内存的函数需要成对出现 | |
| 申请内存后异常退出前需要及时进行内存释放 | |
| 内存申请前必须对内存大小进行合法性校验 | |
| 内存分配后必须判断是否成功 | |
| 禁止使用realloc、alloca函数 | |
| 禁止打印文件路径、口令等敏感信息，如有需要，使用private修饰 | |
| 禁止打印内存地址 | |
| 整数之间运算时必须严格检查，确保不会出现溢出、反转、除0 | |
| 禁止对有符号整数进行位操作符运算 | |
| 禁止对指针进行逻辑或位运算 | |
| 循环次数如果收外部数据控制，需要检验其合法性 | |
| 禁止使用内存操作类危险函数，需要使用安全函数 | |
| 谨慎使用不可重入函数 | |
| 必须检查安全函数的返回值，并进行正确处理 | |
| 禁止仅通过TokenType类型判断绕过权限校验 | |
**TDD Result**:
**XTS Result**:
### 是否已执行L0用例
- [ ] 已验证
- [ ] 不涉及。如不涉及，请写明理由
### AI检视评分（使用本地代码检视skills扫描）:
# 代码检视报告 — PR #20216（Round 1 / 仅 PR 修改内容）
> 统一报告由 codecheck 工作台（orchestrator）生成，**用于门禁管控**。所有 codecheck 报告必须遵循本模板：章节顺序、字段名、报告元数据块、评分与门禁规则均为**固定格式**。
> 本次检视严格遵循用户约束"**仅检视 PR 修改内容**"：仅审查 PR #20216 的提交 `b8a884a` 相对 master(`b354b970`) 的差异；未审查无关代码。本地工作树存在与本 PR 无关的未提交修改（部分文件重叠），故所有文件读取均通过 `git show <SHA>:<path>` 完成，规避工作树污染。
---
## 报告元数据
<!-- codecheck-report-metadata:start -->
```yaml
codecheck_report:
schema_version: "1.0"
scope: "PR #20216 — commit b8a884a (production changes only)"
round: 1
commit_id: "b8a884afa5d287aaeb7a3dabb80bfd1694447dd3"
change_id: "PR-20216 (GitCode, no Change-Id)"
report_id: "b8a884a-R1"
date: "2026-08-16"
gate_decision: "approve"
risk_level: "low"
score: 98
dimensions_required: ["security", "logic", "input"]
dimensions_executed: ["security", "logic", "input"]
findings_total: 1
findings_by_severity: {P0: 0, P1: 0, P2: 0, P3: 1}
gate_blockers: []
must_fix: []
followups: ["LOG-001"]
```
<!-- codecheck-report-metadata:end -->
---
## 1. 门禁结论
| 项目 | 结论 |
|---|---|
| 决策 | **approve** |
| 风险等级 | 🟢 low |
| 评分 | **98/100** |
| 阻塞项 | 无 |
| 必须修复（P0/P1） | 0 项 |
| 建议跟进（P2/P3） | 1 项 |
**一句话结论**：本 PR 是一组连贯的健壮性加固（空指针守卫、IPC Parcel 指针 RAII 化、death-recipient 生命周期修正、Marshalling 返回值校验、死代码移除），未引入可触发缺陷；仅 1 条 P3 潜在设计耦合建议跟进，门禁通过。
### 范围与扫描器组合（Step 2 探测结果）
| 探测信号 | 结果 | 决策依据 |
|---|---|---|
| 路径特征 | `services/abilitymgr/src/`（IPC + 内存缓存）+ `frameworks/native/.../ui_extension_base/` | 服务侧 + IPC |
| IPC 信号 | ✅ `want_receiver_stub.cpp` `ReadParcelable`、`wants_info.cpp` `Marshalling/Unmarshalling` | 触发 input-scanner |
| 持久化信号 | ❌（仅内存 map，无 DB/文件） | input-scanner 仍适用（缓存污染/并发） |
| API 信号 | ❌ 无 `interfaces/kits/` 或 `.d.ts/.h` 签名变更（仅 .cpp 内部守卫） | 按用户"仅检视 PR 修改内容"约束，**跳过 api-scanner** |
| 选定组合 | `security-scanner` + `logic-scanner` + `input-scanner` | 满足 services/+IPC 必检维度 |
### PR 概况
- PR：https://gitcode.com/openharmony/ability_ability_runtime/pull/20216
- 提交：`b8a884afa5d287aaeb7a3dabb80bfd1694447dd3`（"fix bug"，commit message 标注 91% AI 生成）
- 基线：`b354b9709d6b15a64c83f657d0d330da99e45dfe`（master HEAD）
- 变更：15 文件 +679/−96；其中**生产代码 6 文件 +79/−22**，测试 9 文件
- 审查范围：仅 6 个生产文件（测试仅用于印证意图，不作为发现来源）
| 生产文件 | 变更性质 |
|---|---|
| `frameworks/js/napi/ability_manager/js_preload_ui_extension_callback_client.cpp` | +`env_` 空指针守卫 |
| `frameworks/native/ability/native/ui_extension_base/js_ui_extension_context.cpp` | 抽取 `CreateUIServiceExtConnection`（含空检查）、`DoDisconnectUIServiceExtensionComplete`；disconnect 空连接时补 `return;`；`OnReportDrawnCompleted` 补 argc 守卫 |
| `frameworks/native/ability/native/ui_service_extension_ability/js_ui_service_extension_context.cpp` | erase 前补 `RemoveConnectionObject()`（空守卫） |
| `services/abilitymgr/src/ui_extension/ui_extension_ability_manager.cpp` | `RegisterPreloadUIExtensionHostClient` 去重 + AddDeathRecipient 校验 + 异常清理；`CompleteBackground` 移除死代码 `CHECK_POINTER` |
| `services/abilitymgr/src/want_receiver_stub.cpp` | `Want*`/`WantParams*` → `std::unique_ptr`（RAII，异常/早返回安全） |
| `services/abilitymgr/src/wants_info.cpp` | `Marshalling` 校验 `WriteParcelable`/`WriteString16` 返回值 |
---
## 2. 扣分原因
> gate_decision = approve，本节省略。
---
## 3. 必须立即处理（P0/P1）
**无。**
---
## 4. 建议本轮或下一补档处理（P2/P3）
| ID | 优先级 | 问题 | 建议行动 | 排期 |
|---|---|---|---|---|
| LOG-001 | P3 | `RegisterPreloadUIExtensionHostClient` 去重早返回跳过 record-manager token 刷新（潜在设计耦合，当前不可触发） | 二选一：① 去重命中分支仍调用 `recordMgr.Register(callerToken)`（单例下幂等覆盖）；② 在代码注释显式声明 "token-per-pid 稳定" 前置条件 | 下一补档，非阻塞 |
> 另有 1 条**范围外相邻观察** INP-001 见附录 A，不计入本 PR 评分（`ReadFromParcel` 未被本 PR 修改，属既有问题）。
---
## 5. 分维度速览
| 维度 | 结果 | 关键说明 |
|---|---|---|
| security（高影响缺陷/安全） | ✅ 通过（0 发现） | 6 项审查点逐一排除：`AddDeathRecipient` 持锁无重入死锁（OpenHarmony 死亡通知异步派发，与既有 `app_state_observer_manager.cpp` 同模式）；移除的 `CHECK_POINTER(abilityRecord)` 位于函数顶部空守卫(`if (abilityRecord == nullptr) return;`) 之后、且在两次解引用之后，为死代码；`unique_ptr` 生命周期覆盖 `PerformReceive` 全作用域；`env_`/`MakeSptr`/`GetServiceHostStub` 守卫放置正确；`g_connects` 无新增竞态；`Marshalling` 返回值校验正确。 |
| logic（逻辑影响/一致性） | ✅ 通过（1×P3） | 10 项重构等价性评估：R1/R2/R4/R5/R6/R7(除残留)/R8/R9/R10 均为等价或严格 bug-fix；R3 抽取行为等价；R7 去重留 1×P3 潜在耦合（LOG-001）。`DoDisconnectUIServiceExtensionComplete` 与原内联 lambda 行为等价；disconnect 空 `return;` 为 bug 修复（旧代码以 null 调 `DisconnectAbility`），`RemoveUIServiceExtensionConnection` 幂等无新副作用。 |
| input（外部输入→内存缓存/并发） | ✅ 通过（0 计分发现；1 范围外附录） | 确认本 PR 修复 4 处输入健壮性：①`want_receiver_stub` RAII 化（攻击者 parcel 对象异常/泄漏安全）；②`wants_info` 写侧返回值校验；③death-recipient map 持锁+返回校验+异常擦除（无 TOCTOU/死锁，catch 块为新获取锁）；④`g_connects` erase 前 napi-ref 及时释放。`callerPid` 来自 `IPCSkeleton::GetCallingPid`（内核可信，不可伪造）。 |
---
## 6. 关键发现详情
### [LOG-001] RegisterPreloadUIExtensionHostClient 去重早返回跳过 record-manager token 刷新（P3, scanner=logic）
- **位置**：`services/abilitymgr/src/ui_extension/ui_extension_ability_manager.cpp:583`（去重 `return ERR_OK;` 处）
- **触发路径**：
1. 进程 P 首次注册：`preloadUIExtensionHostClientDeathRecipients_[P]=deathRecipient` + `recordMgr.preloadUIExtensionHostClientCallerTokens_[P]=tokenA`。
2. 进程 P 再次注册（无中间 UnRegister，且 tokenA 仍存活）：新代码命中 `find(P)!=end()` → `return ERR_OK`，**跳过** `recordMgr.RegisterPreloadUIExtensionHostClient(callerToken)`。
3. 若新 callerToken 与旧不同，记录管理器保留旧 token。
- **影响（经 refute 修正后）**：当前**不可触发**的功能性后果。
- 生产客户端 `PreloadUIExtensionHostClient` 为进程级单例（`preload_ui_extension_host_client.cpp:51` 传 `GetInstance()`），重复注册传入**同一 token 对象**，跳过覆盖为 no-op，无 token 过期。
- 去重命中前提是旧 deathRecipient 仍在册 ⇒ 旧 token 仍存活 ⇒ 记录管理器旧 token 仍有效，`OnLoadedDone` 等回调经旧 token 仍可触发。原述"OnLoadedDone 永不触发"不成立。
- 残留为**潜在设计耦合**：去重隐含"token-per-pid 稳定"假设，该假设仅因单例客户端成立、IPC 边界未强制；将来若改为 per-ability token 会静默引入过期。
- **证据**：
- `git show b8a884a:services/abilitymgr/src/ui_extension/ui_extension_ability_manager.cpp`（去重分支）
- `services/abilitymgr/src/extension_record/extension_record_manager.cpp:1200-1209`（`preloadUIExtensionHostClientCallerTokens_[callerPid] = callerToken;` 覆写语义）
- `frameworks/native/ability/native/preload_ui_extension_host_client.cpp:51`（`GetInstance()` 单例 token）
- **建议**：二选一——① 去重命中分支仍调用 `recordMgr.Register(callerToken)`（单例下幂等）；② 注释显式声明前置条件。
- **refute 判定**：⬇️ P2 → P3（影响夸大证伪 + 触发不可达，详见 `refute_log.md`）。
---
## 7. 附录
### 附录 A：范围外/相邻观察（不计分）
#### [INP-001] WantsInfo::ReadFromParcel 未校验 ReadString16 失败（P3, 范围外）
- **位置**：`services/abilitymgr/src/wants_info.cpp:30`（`resolvedTypes = Str16ToStr8(data.ReadString16());`）
- **说明**：本 PR 仅硬化了**写侧** `Marshalling`（校验 `WriteParcelable`/`WriteString16` 返回值），**读侧** `ReadFromParcel`/`Unmarshalling` 未被修改且未校验 `ReadString16` 失败——截断入站 parcel 会静默得到 `resolvedTypes==""` 并返回成功，与写侧修复不对称。
- **范围判定**：`ReadFromParcel` **未被本 PR 修改**，属既有问题；按用户"仅检视 PR 修改内容"约束，**不计入本次计分**，仅作相邻观察列出。建议后续单独处理读侧对称校验。
### 附录 B：scanner 已排除/已考虑项（保留供人工捞回）
| 项 | 来源 scanner | 排除理由（证据） |
|---|---|---|
| `AddDeathRecipient` 持锁重入死锁 | security | OpenHarmony 死亡通知异步派发；同模式已见于 `app_state_observer_manager.cpp:1174-1213` 长期在网未死锁 |
| 移除 `CHECK_POINTER(abilityRecord)` 致 NPE | security/logic | `CompleteBackground` 顶部已有 `if (abilityRecord == nullptr) return;`（line 1556）且该 CHECK_POINTER 位于两次解引用之后，为死代码 |
| `want_receiver
See merge request: openharmony/ability_ability_runtime!20216