CCExtractor / CCExtractor/ccextractor

switch_to_next_file() re-enters itself through open(), advancing current_file twice

Đang mở
#2,344 3 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
C
Star
903
Fork
589
Merge trung bình
3 ngày 2 giờ
Pull request đã merge (30 ngày)
10

Mô tả

Split out from #2343, which made the symptom memory-safe without addressing the cause.

## What happens

`switch_to_next_file()` advances `current_file`, then calls `open()`. That call reaches `buffered_read_opt()`, which calls `switch_to_next_file()` **again** when `binary_concat` is set — and that nested call advances `current_file` a second time.

Confirmed with a hardware watchpoint on a single-file run:

```
GUARD: current_file=0 num_input_files=1
BEFORE open: current_file=0
AFTER open: current_file=1 <-- advanced inside open()
```

with the mutation occurring here:

```
#0 ccx_rust::file_functions::file::switch_to_next_file (file.rs:220) <-- current_file += 1
#1 ccx_rust::file_functions::file::buffered_read_opt (file.rs:501)
#2 ccx_rust::demuxer::stream_functions::detect_stream_type
#3 ccx_rust::demuxer::common_types::CcxDemuxer::open
#4 ccxr_demuxer_open → ccx_demuxer_open (ccx_demuxer.c:62)
#5 switch_to_next_file (file_functions.c:181) <-- the outer call
```

So the outer call is an unwitting re-entrant caller of itself, across the C/Rust boundary, sharing one mutable counter.

## Why it matters beyond the crash

#2343 stopped the out-of-bounds read by holding the filename across `open()` instead of re-indexing afterwards. The double advance itself is untouched, and it still means:

- `current_file` no longer identifies the file being processed once `open()` returns
- with several inputs, entries can be stepped over: each outer iteration consumes two increments, so which files actually get opened depends on how many times the nested path fires
- two implementations of the same function — `file_functions.c:116` and `src/rust/src/file_functions/file.rs:150` — are both live, both mutate the same state, and neither expects the other

A caller cannot currently tell whether `current_file` means "the file I am opening" or "the file the nested call moved on to". That ambiguity is the actual defect.

## Worth deciding

1. Should `detect_stream_type`'s probing read be able to switch input files at all? Advancing the input list while establishing what the *current* file is looks wrong regardless of the index bug.
2. Should the C `switch_to_next_file()` remain, given the Rust port is what runs under `DISABLE_RUST`-off builds? Two live copies of this logic is how the identical stale-index bug came to exist in both.

## Reproducing

The watchpoint trace above comes from:

```bash
gdb --args ./ccextractor sample.rcwt --out=webvtt --timestamp-map -o out.vtt
break file_functions.c:181
run
watch ctx->current_file
continue
```

The 153-byte RCWT sample from #2342 is enough; no special input is needed.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.