From 5dee57eeed4367f58715ebffa73641beabf4b40c Mon Sep 17 00:00:00 2001 From: xiaoxia Date: Tue, 14 Jul 2026 15:06:16 +0800 Subject: [PATCH] docs: add PR312 security audit report --- docs/PR312多轨道字幕拼接审计报告_20260714.md | 168 +++++++++++++++++++ 1 file changed, 168 insertions(+) create mode 100644 docs/PR312多轨道字幕拼接审计报告_20260714.md diff --git a/docs/PR312多轨道字幕拼接审计报告_20260714.md b/docs/PR312多轨道字幕拼接审计报告_20260714.md new file mode 100644 index 000000000..59476fef8 --- /dev/null +++ b/docs/PR312多轨道字幕拼接审计报告_20260714.md @@ -0,0 +1,168 @@ +# PR #312 多轨道混音+字幕渲染+视频拼接 审计报告 + +## 总览 + +- **结论**：⚠️ 有条件通过（需修复 P1 问题后方可合并） +- **问题统计**：P0 0 项，P1 4 项，P2 9 项，P3 5 项 +- **改动范围**：6 个文件，+2614 行，-2 行 +- **PR标题**：feat: 多轨道混音 + 字幕渲染引擎 + 视频拼接（后端） +- **改动文件**： + - `apps/worker/video_processing/multi_track_mixer.py`（新增，+392 行） + - `apps/worker/video_processing/subtitle_render_engine.py`（新增，+636 行） + - `apps/worker/video_processing/concat_engine.py`（新增，+642 行） + - `apps/worker/video_processing/render_audio.py`（修改，+19/-2） + - `apps/worker/video_processing/unified_render_service.py`（修改，+2/-0） + - `tests/unit/test_multi_track_subtitle_concat.py`（新增，+923 行） + +## 三大重点审计结论 + +### 1. FFmpeg 滤镜注入风险 + +**总体评价**：✅ 低风险 + +- ✅ 所有 FFmpeg 调用均通过 `run_ffmpeg` 使用列表参数传递（`subprocess.run` with list args），无 `shell=True`，不存在命令注入风险 +- ✅ 数字参数（音量、时长、坐标等）均经过 `float()` / `int()` 转换，并设有范围钳制（如 `volume` 限制在 0.0~2.0） +- ✅ 多轨道混音的 `filter_complex` 由受控的数值和内部标签拼接，标签名（`v{i}_in`、`a{i}_in` 等）由代码生成，不可控 +- ⚠️ `build_subtitle_filter` 的路径转义不完整（未转义 `[` `]`），但当前仅用于服务器生成的路径，风险较低 +- ⚠️ concat demuxer 的 list 文件中路径仅转义单引号，虽不直接导致命令注入（走 list 文件而非 shell），但若路径中含换行符可能拼接新的 file 行 + +### 2. 字幕文件路径安全 + +**总体评价**：⚠️ 中等风险（路径来源需关注） + +- ✅ ASS 文件生成位置在 `work_dir` 内（服务器控制），文件名由 `plan_id` 构成，无路径遍历 +- ✅ ASS 文本内容经过 `_escape_ass_text` 转义（大括号→圆括号、换行→\N），防止 ASS 覆盖标签注入 +- ✅ 颜色值经过 `_hex_to_ass_color` 校验（非 6 位 hex 降级为白色） +- ⚠️ `build_subtitle_filter` 中的字幕文件路径若来自用户输入，可能存在 filter 语法注入风险 +- ⚠️ 字幕文件无大小上限（极端场景下大量字幕片段可能生成超大 ASS 文件） +- ❌ 新增模块未接入任何 `path_security` 校验（注：当前代码库中尚未发现 path_security.py 模块，仅在 __pycache__ 中有缓存文件） + +### 3. 拼接资源消耗控制 + +**总体评价**：❌ 高风险（缺少关键限制） + +- ❌ **拼接段数无上限**：`ConcatConfig.from_config_dict` 不限制 segments 数量，大量片段拼接会消耗大量内存和 CPU +- ❌ **轨道数量无上限**：`MultiTrackMixConfig.from_config_dict` 不限制 tracks 数量 +- ❌ **无总时长上限**：拼接后总时长没有任何限制 +- ❌ **无单段文件大小限制**：输入文件大小没有校验 +- ⚠️ 使用 concat filter 模式时，所有输入文件同时解码，内存占用与段数成正比 +- ✅ 有 30 分钟的 FFmpeg 超时机制（`DEFAULT_FFMPEG_TIMEOUT = 1800`） +- ✅ worker 级别有 4 并发限制（`worker_concurrency: int = 4`） +- ⚠️ 临时文件（track 预处理文件、concat_list.txt）使用后未清理 + +## 问题清单（按等级） + +### P0 + +无。所有 FFmpeg 调用均使用列表参数，不存在直接的命令注入漏洞。 + +### P1 + +1. **【multi_track_mixer.py】audio_path 无路径安全校验** + - 位置：`AudioTrack.from_dict`（L98）及 `_prepare_single_track`（L173/L240） + - 描述：`audio_path` 直接从用户配置中读取字符串，未经过任何路径校验（如路径白名单、路径遍历检测）。用户可通过 `plan.config.audio_tracks.tracks[].audio_path` 传入任意文件路径，ffprobe/ffmpeg 会读取该文件。虽不直接执行命令，但可探测服务器上任意文件的存在性，且音频/视频文件会被转码后进入输出流。 + - 风险：任意文件读取/探测（信息泄露） + - 建议：参照项目既有的 `asset_id → local_path` 解析模式，或引入 `path_security` 模块校验路径必须在指定目录内 + +2. **【concat_engine.py】video_path 无路径安全校验** + - 位置：`ConcatSegment.from_dict`（L72）及 `_concat_filter`（L319）、`_concat_demuxer`（L281） + - 描述：`video_path` 直接从配置读取，无任何路径安全校验。同理，用户可通过拼接配置读取服务器任意文件。 + - 风险：任意文件读取/探测（信息泄露） + - 建议：同上，增加路径安全校验 + +3. **【multi_track_mixer.py】无轨道数量上限** + - 位置：`MultiTrackMixConfig.from_config_dict`（L119-L137） + - 描述：`tracks` 列表无数量上限校验，用户可传入成百上千个轨道，导致 FFmpeg 同时打开大量文件、构建复杂 filter_complex，消耗大量 CPU/内存/文件描述符。 + - 风险：资源耗尽（DoS） + - 建议：设置合理上限（如 `MAX_TRACKS = 16`），超过则截断并记录告警 + +4. **【concat_engine.py】无拼接段数上限** + - 位置：`ConcatConfig.from_config_dict`（L99-L113） + - 描述：`segments` 列表无数量上限校验。concat filter 模式下所有输入文件同时解码，内存占用线性增长。 + - 风险：资源耗尽（DoS） + - 建议：设置合理上限（如 `MAX_SEGMENTS = 50`），超过则截断或报错 + +### P2 + +5. **【concat_engine.py】无总时长/单段时长上限** + - 描述：没有限制拼接后总时长或单段时长，超长视频拼接会长时间占用 worker。 + - 建议：增加 `MAX_TOTAL_DURATION`（如 2 小时）和 `MAX_SEGMENT_DURATION` 限制 + +6. **【multi_track_mixer.py】预处理临时文件未清理** + - 位置：`mix_multi_track` 方法（L303） + - 描述：`track_{i}_{plan_id}.aac` 等临时文件在混音完成后未被清理，长期运行会占用磁盘空间。 + - 建议：混音完成后 `try/finally` 清理临时轨道文件 + +7. **【concat_engine.py】concat_list.txt 未清理** + - 位置：`_concat_demuxer`（L275） + - 描述：生成的 concat demuxer 列表文件使用后不删除。 + - 建议：使用完毕后删除列表文件 + +8. **【concat_engine.py】transition 字段无白名单校验** + - 位置：`ConcatConfig.from_config_dict`（L132） + - 描述：`transition = str(config.get("transition", "none"))` 直接转字符串，无白名单验证。虽然目前仅判断 `!= "none"` 时禁用 stream copy，但未来扩展转场效果（如 xfade）时，transition 名称会进入 filter_complex，存在注入风险。 + - 建议：使用枚举或白名单列表校验 transition 值 + +9. **【subtitle_render_engine.py】字幕滤镜路径转义不完整** + - 位置：`build_subtitle_filter`（L633-L636） + - 描述：仅转义了 `\`、`:`、`'`，未转义 `[`、`]`。若字幕文件路径中包含方括号，可能破坏 filter_complex 语法导致 FFmpeg 报错。 + - 当前风险：低（路径由服务器生成，不含方括号） + - 建议：补充 `[` → `\\[`、`]` → `\\]` 的转义 + +10. **【subtitle_render_engine.py】字幕片段数量/文件大小无上限** + - 描述：ASS 文件可包含无限多字幕片段，极端场景下可能生成超大文件。 + - 建议：增加 `MAX_SEGMENTS` 限制（如 10000 条） + +11. **【concat_engine.py】_concat_filter 大量死代码** + - 位置：`_concat_filter` 方法 L330-L418 + - 描述：方法前半部分（约 100 行）构建的 `filter_parts` 和 `concat_inputs` 在 L418 被 `filter_parts.clear()` 全部清除，然后从 L420 重新构建。前半部分是完全无用的死代码，增加维护成本且容易误读。 + - 建议：删除前半部分死代码，保留清晰的实现 + +12. **【multi_track_mixer.py】track_id/track_type 无字符校验（日志注入）** + - 描述：`track_id` 和 `track_type` 是用户可控字符串，直接用于日志输出，存在日志注入风险（如插入伪造日志行）。 + - 建议：对日志输出的字符串做长度和换行符限制 + +13. **【API 层】新增配置字段无 schema 校验** + - 位置：`packages/domain/config_schemas.py` 的 `normalize_plan_config` + - 描述：`normalize_plan_config` 仅校验和清洗 `cover/title/subtitle/bgm/editing_mode/transition_enabled` 字段。新增加的 `audio_tracks`、`manual_subtitles`、`title_config`、`subtitle_config`（新格式）等字段未纳入校验，任意 JSON 都可以透传到 worker。 + - 建议：在 config_schemas 中新增对应的数据模型和校验逻辑 + +### P3 + +14. **【concat_engine.py】未使用的导入** + - `import tempfile` 和 `from typing import Any` 未使用 + +15. **【subtitle_render_engine.py】未使用的导入** + - `dataclasses.field`、`generate_ass_subtitles`、`generate_ass_from_timeline` 未使用（预留导入） + +16. **【subtitle_render_engine.py】尚未集成到主渲染流程** + - `SubtitleRenderEngine` 和 `build_subtitles_from_plan` 是独立模块，`unified_render_service.py` 仍使用旧的 `render_subtitles.py` 和 `subtitle_generator.py`，新引擎暂未接入 + +17. **【concat_engine.py】尚未集成到主渲染流程** + - `ConcatEngine` 仅提供独立的拼接能力，未接入 `UnifiedRenderService` 的渲染管线 + +18. **【concat_engine.py】transition 文档标注"预留"但已有逻辑判断** + - 字段注释写着"转场效果（none/crossfade）- 预留"，但 `_can_use_stream_copy` 中已有 `if config.transition != "none": return False` 的判断，建议统一描述 + +## 亮点 + +1. **优秀的降级策略**：三个模块均有完善的降级/容错机制——单轨失败跳过、混音失败回退主音频、参数不一致自动降级到 filter 模式、配置无效返回 None 等，符合"能力不可用时不阻断主流程"的设计原则 +2. **完善的参数安全转换**：所有数字参数都经过 try/except 包裹的类型转换，失败时降级到默认值；音量、不透明度等有范围钳制 +3. **统一使用 run_ffmpeg**：所有 FFmpeg 调用均通过 `run_ffmpeg` 工具函数，使用列表参数传递，从根本上避免 shell 注入 +4. **单测覆盖充分**：52 个新增单测覆盖了配置解析、正常流程、异常场景、降级路径，完整度较高 +5. **代码结构清晰**：三个模块均遵循统一的风格（dataclass + from_dict + 引擎类 + 便捷函数），符合 UnifiedRenderService 架构风格 +6. **ASS 文本转义到位**：`_escape_ass_text` 正确转义了大括号（防止 ASS 覆盖标签注入）和换行符 + +## 修复建议（优先级排序） + +1. **【P1】** 为 `audio_path` 和 `video_path` 增加路径安全校验（接入 path_security 模块或使用 asset_id 解析模式） +2. **【P1】** 增加轨道数量上限（建议 `MAX_TRACKS = 16`） +3. **【P1】** 增加拼接段数上限（建议 `MAX_SEGMENTS = 50`） +4. **【P2】** 增加拼接总时长上限和单段时长上限 +5. **【P2】** 清理临时文件（预处理轨道文件、concat_list.txt） +6. **【P2】** `transition` 字段增加白名单校验 +7. **【P2】** 删除 `_concat_filter` 中的死代码 +8. **【P2】** 字幕滤镜路径转义补充 `[` `]` +9. **【P2】** API 层 config_schemas 增加新配置字段的校验模型 +10. **【P3】** 清理未使用的导入 +11. **【P3】** 字幕片段数量增加合理上限