[BUG] MXF: update_tid_lut() writes past tracks[32] onto nb_tracks, turning a file-supplied track id into the write offset

未关闭 适合新手
  • 哪里不对？
#2,457 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

维护者通常 7 天内回复

@pranayr710 已经在做这个了。

开始于 2026年11月10日。

  • #2458 来自 @pranayr710 —— 未关闭

评估

难度
2/5
预计耗时
1-3 小时
新手友好度
65/100
Issue 类型
缺陷
描述清晰度
描述清楚
活跃度
活跃
技术栈
c
领域
backend, security

调研方向

从 src/lib_ccx/ccx_demuxer_mxf.c 中的 update_tid_lut() 入手，检查在对已满的 tracks 数组进行索引之前如何处理它。阅读调用方和遍历轨道数量的循环，以确认上限行为，然后构建并测试一个声明了超过 32 条轨道的 MXF 文件；当超出的轨道被安全忽略，且轨道计数和相邻字段保持完整时，即为完成。

由索引模型根据 Issue 内容生成。

描述

Summary

update_tid_lut() in src/lib_ccx/ccx_demuxer_mxf.c appends to a fixed
MXFTrack tracks[32] without checking nb_tracks first. nb_tracks is the
struct field directly behind that array, so the 33rd track a file declares
overwrites the bound itself with a 32-bit value taken from the file — and the
very next statement re-reads that bound to choose where to write.

The overlap is exact
typedef struct MXFContext
{
        enum MXFCaptionType type;        /* byte   0 */
        int cap_track_id;                /* byte   4 */
        UID cap_essence_key;             /* byte   8 */
        MXFTrack tracks[32];             /* byte  24 .. 279 */
        int nb_tracks;                   /* byte 280 */
        int cap_count;                   /* byte 284 */
        struct ccx_rational edit_rate;   /* byte 288 */
} MXFContext;                            /* 296 bytes, heap allocated in ccx_mxf_init() */

sizeof(MXFTrack) is 8, so tracks[32] begins at 24 + 32*8 = 280 — the
same address as nb_tracks.

What happens on the 33rd track
ctx->tracks[ctx->nb_tracks].track_id = track_id;
memcpy(ctx->tracks[ctx->nb_tracks].track_number, track_number, 4);
ctx->edit_rate = edit_rate;
ctx->nb_tracks++;
  • statement 1 writes track_id at byte 280, i.e. into nb_tracks;
  • statement 2 re-evaluates ctx->tracks[ctx->nb_tracks] using the value just
    written, so its destination is 24 + 8*track_id + 4.

track_id is read straight from the file in mxf_read_timeline_track_metadata():

case MXF_TAG_TRACK_ID:
        track_id = buffered_get_be32(demux);

With track_id = 0x7FFFFFF0 the memcpy targets byte 17179869084 of a 296
byte allocation — about 16 GB past it. Smaller ids give smaller, more usable
offsets; the write is at 8 * track_id bytes from the block, with four bytes of
file-supplied content.

Everything afterwards is affected too: every for (i = 0; i < ctx->nb_tracks; i++) over tracks[] (here and in update_cap_essence_key()) becomes an
unbounded read, and cap_count at byte 284 is overwritten by track_number.

Reachability

ccx_probe_mxf() scans the first bytes for the 14-byte header partition pack
key and nothing else, so any file carrying that key is handed to this demuxer.
Each Timeline Track set in the header metadata calls update_tid_lut() once —
33 of them is enough. The C path is live; the Rust side only re-exports
ccx_probe_mxf / ccx_mxf_init ("shall be removed after mxf" in
src/rust/build.rs).

Demonstrating the mechanism

To be clear about what this evidence is and is not: the run below is a
standalone program that reproduces MXFContext and update_tid_lut()
verbatim, with no other CCExtractor code. It demonstrates the pointer
arithmetic and the crash; it is not CCExtractor itself crashing. My sanitizer
build is unavailable at the moment, so I have not yet put the crafted .mxf
files through the real binary end to end — I will add that as a comment rather
than claim it now. The struct offsets below are computed from the real
declaration, and the reachability argument is from the source.

Reproducing the exact struct and function (no other CCExtractor code):

MASTER build
  after 32 tracks: nb_tracks = 32
  adding 33rd track, track_id = 0x7FFFFFF0 ...
  Segmentation fault

FIXED build
  after 32 tracks: nb_tracks = 32
  adding 33rd track, track_id = 0x7FFFFFF0 ...
    [guard] table full at 32, ignoring track 0x7FFFFFF0
  survived: nb_tracks = 32, cap_count = 0x00000000

and the intermediate state, printed between the two statements:

nb_tracks before        = 32
nb_tracks after stmt 1  = 0x7FFFFFF0   <-- clobbered by the file's track_id
statement 2 will memcpy to tracks[2147483632].track_number = byte 17179869084 of a 296 byte block
Suggested fix

Refuse to append once the table is full:

if (ctx->nb_tracks >= (int)(sizeof(ctx->tracks) / sizeof(ctx->tracks[0])))
{
        mprint("MXF: more than %d tracks, ignoring the rest\n", ...);
        return;
}

I have a patch and a generator for the test files, and will open a PR
referencing this issue.

(Analysis and patch prepared with AI assistance; the reproducers are my own.)

主要语言
C
星标
913
派生
603
平均合并
6 天 8 小时
30 天内合并 PR
33

环境准备

从这里开始

  1. 先读完整个 Issue，再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库，在一个分支上完成修改。
  4. 提交 Pull Request，并在描述里引用这个 Issue 编号。

CCExtractor/ccextractor 的其他 Issue

查看 CCExtractor/ccextractor 的全部 Issue

相似的 Issue

更多 C Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。