CCExtractor / CCExtractor/ccextractor

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

Ouverte
#2,344 3 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
Langage dominant
C
Étoiles
903
Forks
589
Merge moyen
3 j 2 h
PR mergées (30 j)
10

Description

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.

Guide de contribution

Ouvrir le guide de contribution

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.