Files
dstalk/agents/audits/W13.4-lsp-audit.md
XiuChengWu 47082376ef
Some checks failed
CI / Determine matrix (push) Has been cancelled
CI / ${{ matrix.os }} / ${{ matrix.build_type }} (push) Has been cancelled
Wave 10: deep audits of 5 unaudited plugins, smoke regression set (W13.1-W13.6)
- W13.1 anthropic_plugin (architect-yang, 497 lines): rated C. 6 C ABI
  functions lack try/catch (§8 violation); my_chat leaks response_body on
  error path; tool_use response silently dropped.
- W13.2 deepseek_plugin (engineer-sun, 486 lines): rated C+. 7 ABI entries
  unprotected including json::parse paths (malformed JSON terminates);
  SSE [DONE] sentinel match brittle; ~55% code overlap with anthropic
  suggests an ai_plugin_base extraction.
- W13.3 network_plugin (qa-wang, 322 lines): rated C. CRITICAL: TLS
  certificate verification fully disabled (set_verify_mode never called,
  default verify_none accepts any cert) — all AI traffic incl. api_key
  is MITM-vulnerable. DNS resolve has no timeout; catch lacks (...).
- W13.4 lsp_plugin (architect-huang, 749 lines): rated C. CRITICAL:
  guaranteed deadlock at L519-526 → L547 (g_lsp_impl_start holds mutex
  then calls g_lsp_impl_stop which re-locks the same non-recursive
  mutex); 7 vtable funcs unprotected; server→client requests dropped.
- W13.5 session+tools (security-cao, 264+251 lines): rated D+/D. Path
  traversal in builtin_file_read/write (zero validation); global
  static state in both plugins lacks mutex (UAF risk); 9 vtable funcs
  lack try/catch.
- W13.6 smoke regression (qa-xu, +193 lines): 4 new cases — context
  max_tokens trim, config dual-store consistency (exposes that W12.2
  merge is incomplete: dstalk_config_set→config_service.get returns
  null), HTTP error path no-crash, repeated init/shutdown cycle.

Verified: cmake build 0 error 0 warning, ctest 4/4 pass.

Top W14 priorities surfaced: TLS verification (W13.3), LSP deadlock
(W13.4), file-tool path traversal (W13.5), config dual-store still
broken (W13.6 R2), shared try/catch wrapper across all AI plugins.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2026-05-27 09:32:13 +08:00

270 lines
12 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# W13.4 LSP Plugin 深度审计
**Auditor**: 黄岭 (architect-huang)
**Date**: 2026-05-27
**File**: plugins/lsp/src/lsp_plugin.cpp (749 行)
**Wave Coverage**: W6.1 仅审计 reader_loop (L415-459),其余 700+ 行首次审计
---
## 0. 文件总览
749 行。模块分布:
- Process 子进程 (start/stop/write/read_line/read_bytes): ~220 行 (L54-274)
- Service dispatch 实现 (start/stop/open/close/diagnostics/hover/completion): ~280 行 (L461-698)
- JSON-RPC 框架 (frame_message/parse_content_length/send_request/send_notification/handle_message): ~110 行 (L299-413)
- reader_loop 状态机 (W6.1 已修): ~45 行 (L415-459)
- State/Lifecycle/Descriptor: ~50 行 (L276-297 + L700-749)
- Headers/Globals: ~44 行 (L1-53)
## 1. §5 + §8 异常安全——穿越 ABI 边界
状态: **严重违反** FAIL
vtable 全部 7 个函数均以 C 函数指针暴露给 host (`dstalk_lsp_service_t`),但底层 C++ 实现大量使用 `std::string` / `json::object` / `json::parse` / `std::unique_lock` 等可抛异常类型,**零 try/catch 保护**:
| 函数 | 行号 | 可抛异常操作 |
|------|------|-------------|
| `g_lsp_impl_start` | L467-537 | json::object, std::string, std::thread, std::unique_lock |
| `g_lsp_impl_stop` | L539-566 | json::object, std::unique_lock |
| `g_lsp_impl_open_document` | L568-584 | json::object |
| `g_lsp_impl_close_document` | L586-598 | json::object |
| `g_lsp_impl_get_diagnostics` | L600-612 | std::lock_guard, std::string |
| `g_lsp_impl_get_hover` | L614-655 | json::object, json::parse, std::unique_lock, std::string |
| `g_lsp_impl_get_completion` | L657-698 | 同上 |
此外 `reader_loop` (L419-459)、`handle_message` (L375-413)、`on_shutdown` (L724-730) 均无保护。
**违反代价**: OOM 或任何 `boost::json` 内部异常穿越 C 函数指针边界 → `std::terminate()` → 进程崩溃零恢复机会。plugin-abi §8 标记此为强制规则 (§8.1 明确覆盖 service vtable 函数指针),当前代码全线违规。
对比 W11.1: context_plugin 的 trim_impl (L114-226) 同样违规,但当时仅 1 个 vtable 函数受影响。lsp_plugin 受影响面是其 7 倍。
评级: **F**
---
## 2. §9 字符串返回值生命周期
状态: **合规** PASS
三个返回 `char** json_out` 的函数:
- `g_lsp_impl_get_diagnostics` (L600-612): `g_host->strdup(it->second.c_str())``g_host->strdup("[]")`,均在 mutex 锁内复制。Mode A (§9.2),调用方 `host->free` 释放。合规。
- `g_lsp_impl_get_hover` (L614-655): `g_host->strdup(json::serialize(resp["result"]).c_str())`temporary 在 strdup 复制后销毁,指针独立。合规。
- `g_lsp_impl_get_completion` (L657-698): 同 get_hover。合规。
`dstalk_plugin_info_t` 的 name/version/description 均为 static 字面量 → Mode B。合规。
评级: **A**
---
## 3. §2 跨 DLL 堆纪律
状态: **合规** PASS
逐行检查所有 malloc/free/strdup/new/delete:
| 模式 | 结果 |
|------|------|
| 裸 `malloc`/`free`/`new`/`delete` | 0 处 |
| 裸 `strdup` | 1 处 (L143, POSIX 子进程内, fork 后 exec 前, 不跨 DLL) |
| `g_host->strdup` | 4 处 (L607/609/653/696), 全部正确 |
| `g_host->alloc`/`g_host->free` | 0 处 |
唯一裸 `strdup` 位于 `fork()` 后的子进程 (L127-151),该进程随即调用 `execvp()` 替换地址空间,或 `_exit(127)` 退出。不涉及跨 DLL 边界。无合规风险。
评级: **A**
---
## 4. 进程生命周期
状态: **多个问题** WARN
### 4.1 Win32: TerminateProcess 无条件调用 (L174-176)
```cpp
WaitForSingleObject(hProcess, 2000); // 返回值被忽略
TerminateProcess(hProcess, 1); // 无条件调用
```
即使进程在 2 秒内正常退出,`TerminateProcess` 仍被调用。虽对已退出进程无害(返回 ERROR_ACCESS_DENIED但语义错误且日志缺失。
### 4.2 POSIX: SIGKILL 后 waitpid 可能永久阻塞 (L191-192)
```cpp
kill(pid, SIGKILL);
waitpid(pid, &status, 0); // 阻塞等待,无超时
```
若子进程处于 D 状态(不可中断睡眠),`waitpid(0)` 永不返回 → 调用线程永久挂起。
### 4.3 POSIX 子进程: strdup 未检查 null (L143-144)
`strdup(cmd)` 在 fork 后调用,若返回 NULL → `strtok(NULL, ...)` → 段错误。虽在 exec 前,但仍是未处理失败路径。
### 4.4 start() 中不必要调用 stop() (L69)
`Process::start` 开头调用 `stop()`,后者在 Win32 上有 2 秒 `WaitForSingleObject`。若首次启动,句柄均为 INVALID无实际操作。但语义不清`stop()` 应独立于 `start()` 调用。
评级: **C**
---
## 5. JSON-RPC 协议合规
状态: **部分缺失** WARN
### 5.1 Content-Length 解析 (L323-341)
W6.1 已修。大小写不敏感、空白跳过、异常安全。合规。
### 5.2 消息分发缺口 (L384-412)
当前仅处理两种消息:
- Response: `id && !method` (L384)
- Notification: `method && !id` (L391)
**缺失**: Server→Client Request (`id && method`) 被静默忽略。LSP 规范中 server 可发送:
- `window/showMessageRequest` (需客户端响应)
- `workspace/applyEdit` (需客户端响应)
- `client/registerCapability`
- `workspace/configuration`
server 请求得不到响应可能导致 LS 功能降级或阻塞。
### 5.3 Error 对象未处理 (L651, L694)
`get_hover` / `get_completion` 仅检查 `resp.contains("result")`,不检查 `"error"`。若 server 返回 `{"jsonrpc":"2.0","id":N,"error":{"code":-32601,"message":"..."}}`,函数返回 -1 但 **丢弃错误详情**。调用方无法区分超时和 server 错误。
### 5.4 通知/响应的 ID 匹配 (L386, L388)
`as_int64()``static_cast<int>`: 若 server 回传 >INT_MAX 的 id 值则截断。不过当前 id 由本端控制,仅 server→client request 场景可能触发。
评级: **B**
---
## 6. 状态机 / 线程同步
状态: **死锁 + 数据竞争** FAIL
### 6.1 致命死锁: start 超时路径 (L519-L526 → L547)
```
L519: std::unique_lock<std::mutex> lock(g_lsp.mutex); // 获取 mutex
L520: g_lsp.cv.wait_for(lock, 10s, ...); // 超时返回lock 仍持有
L526: g_lsp_impl_stop(); // → 进入 g_lsp_impl_stop
L547: std::unique_lock<std::mutex> lock(g_lsp.mutex); // 再次 lock 同一非递归 mutex → 死锁!
```
`std::mutex` 非递归,同线程二次 lock 为未定义行为Windows/Linux 均死锁。initialize 超时 (10s) 后触发,调用线程永久挂起。
注意: `get_hover`/`get_completion` 超时时仅返回 -1不调 stop无此问题。
### 6.2 写管道无同步 (L357, L368)
`send_request``send_notification` 均调用 `proc.write()` 而无 mutex 保护。若多线程并发调用 LSP 服务 → JSON 帧在管道上交错 → 协议解析失败。
### 6.3 读管道关闭的竞态 (L558-562 vs L228)
`g_lsp_impl_stop` 的时序: L558 `running=false` → L559 `proc.stop()` (关闭 hStdOut) → L562 `join`。reader 线程可能正阻塞于 `ReadFile(hStdOut)` (L228),句柄关闭触发 FALSE 返回 → `read_line` 返回 false → loop 退出 → join 成功。此竞态**被利用**来唤醒 reader属有意设计。可工作但脆弱。
评级: **F**
---
## 7. 请求 ID 管理
状态: **潜在 UB** WARN
- L284: `std::atomic<int> next_id{1};`
- L348: `int id = g_lsp.next_id.fetch_add(1);`
- L484: `g_lsp.next_id = 1;` (重启用)
问题:
1. **fetch_add 溢出**: `std::atomic<int>::fetch_add` 在有符号整型溢出时是未定义行为 (C++ 标准 [atomics.types.int]/8)。虽 2^31 次请求极难达到,但长生命期 LS 进程不排除。
2. **重启安全**: L484 重置为 1 时旧 pending 已在 L551 清除,安全。
3. **窄化转换**: L386 `as_int64()``int` 截断(见 §5.4)。
评级: **B**
---
## 8. 资源清理
状态: **基本正确** OK
`on_shutdown` (L724-730): 检查 running → 调 `g_lsp_impl_stop` → 置 `g_host=nullptr`。路径正确。
`g_lsp_impl_stop` (L539-566):
1. shutdown request + 2s wait
2. exit notification
3. `running = false`
4. `proc.stop()` (terminate/kill 子进程)
5. `reader_thread.join()`
6. `diagnostics.clear()`
时序正确。pending_responses 在 L551 清除后,任何在 `cv.wait_for` 阻塞的调用者因 `!g_lsp.running` 被唤醒并返回 -1。无泄漏。
`g_host``on_shutdown` 与 service 函数间无同步 (同 W11.1 的 Race A)。缓解: host 保证 shutdown 前无 in-flight 调用。
评级: **B**
---
## 9. 能力协商
状态: **不完整** WARN
- `initialize` 请求中声明 hover/completion/diagnostic 能力 (L491-503)。合规。
- `initialized` 通知在 initialize 响应后发送 (L533)。符合 LSP spec。
- **ServerCapabilities 被丢弃** (L529): initialize 响应仅取出后删除server 上报的能力集完全未被读取。若 server 不支持 hover插件不会知道仍会发送 hover 请求。
- **rootUri 硬编码 nullptr** (L511): 多数 LS 需要 rootUri 提供项目根目录以建立索引。缺少 rootUri 可导致 go-to-definition、workspace symbols 等完全不工作。
评级: **C**
---
## TOP 3 严重问题
### 1 — [严重] initialize 超时死锁 (L519-526 → L547)
`g_lsp_impl_start` 持有 `g_lsp.mutex` (L519) 时调用 `g_lsp_impl_stop()` (L526),后者在 L547 再次 `unique_lock` 同一非递归 mutex → **自死锁,线程永久挂起**。任何 initialize 超时场景server 10s 不响应)触发。
### 2 — [严重] 全 vtable 无异常保护,违反 §8 (L467-698 + L724-730)
7 个 service vtable 函数 + `reader_loop` + `handle_message` + `on_shutdown` 均大量使用 `std::string` / `json::object` / `json::parse`,零 `try/catch`。OOM 或 boost::json 异常穿越 C 函数指针边界 → `std::terminate()` 进程崩溃。受影响面是 context_plugin (W11.1) 的 7 倍。
### 3 — [高] Server→Client Request 被静默丢弃 + error 信息丢失 (L384-412 + L651/L694)
`handle_message` 仅分发 response/notification`id && method` 的 server request 直接丢弃不响应。server 可能因此阻塞(如 `window/showMessageRequest` 等待用户选择)。同时 error response 的 `error` 字段被忽略 (L651/L694),调用方无法区分超时与 server 错误。
---
## 整体评级
| 维度 | 评级 |
|------|------|
| §5+§8 异常安全 (ABI) | F (全 vtable 裸奔) |
| §9 字符串返回 | A (完全合规) |
| §2 跨 DLL 堆纪律 | A (0 处违规) |
| 进程生命周期 | C (死锁 + waitpid 阻塞 + strdup 未检查) |
| JSON-RPC 协议合规 | B (分发缺口 + error 丢弃) |
| 状态机 / 线程同步 | F (死锁 + pipe write 竞态) |
| 请求 ID 管理 | B (fetch_add 溢出 UB) |
| 资源清理 | B (路径正确,缺同步) |
| 能力协商 | C (ServerCapabilities 丢弃rootUri 硬编码 null) |
| **综合** | **C** |
**总评**: lsp_plugin 在堆纪律和字符串生命周期上完全合规(比 context_plugin 更干净),但 C++ 异常安全全线崩溃7 个 vtable + 多个内部函数无保护)。最致命的是 initialize 超时自死锁——触发条件明确且结果确定永久挂起。JSON-RPC 协议处理不完整 (server request 丢弃) 在真实 LS 场景下会显著降级功能。这些问题集中在 280 行 dispatch 代码中,修复范围可控(约 300 行需加 try/catch 包裹 + 修复死锁 + 补充分发逻辑)。
---
## 补充发现 (优先级低)
- **`#include <boost/json/src.hpp>` (L12)**: 将整个 Boost.JSON 实现编译进插件 DLL增加编译时间且可能与其他插件/库的 Boost.JSON 符号冲突。应改为仅 `#include <boost/json.hpp>`,通过 `target_link_libraries` 链接 boost::boost (CMakeLists.txt L16 已链接)。
- **trim (L303-311) 与 context_plugin 重复**: 相同逻辑出现于 context_plugin 的 trim两处独立维护。建议提取到 dstalk 核心库。
- **MultiByteToWideChar 缓冲区硬编码 4096 (L95)**: 命令行超长时静默截断,进程可能无法正确启动。
- **JSON-RPC "id" 同时接受 null**: LSP 规范允许 `"id": null` 表示通知。当前用 `msg.contains("id")` 仅检查 key 存在,未检查 null 值。若 server 误发 null-id response会被当作有效响应。暂非实际问题主流 LS 实现均守约)。
- **initialize 超时后 g_lsp_impl_stop 仍发 shutdown/exit 给可能未初始化的 LS**: 若 server 启动但未完成 initialize 握手,收到 shutdown 行为未定义。但在实践中,多数 LS 仍能处理。
- **Process::read_bytes L249 `buf.resize(count+1)``buf[count]='\0'`**: 刻意添加 null terminator`std::string` 不要求 null 终止,但后续 `json::parse(buf)` 需要。为性能优化(避免额外拷贝),可接受。