CCExtractor / CCExtractor/ccextractor
switch_to_next_file() re-enters itself through open(), advancing current_file twice
- 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
Đánh giá
Issue này chưa được đánh giá.