Skip to content

[BUG] write_webvtt_header(): --webvtt-create-css leaks basefilename, and re-emits the header before every cue when the CSS file can't be created #2338

Description

@pranayr710

CCExtractor version: master (907be05)

Necessary information

  • Is this a regression (i.e. did it work before)? NO — present since --webvtt-create-css was added.
  • What platform did you use? All (platform-independent; this is a logic bug, not a portability one)
  • What were the used arguments? --out=webvtt --webvtt-create-css

Video links

Not needed — any input that produces WebVTT output reproduces this. The trigger is the output directory, not the input file. Steps are below.

Additional information

write_webvtt_header() in src/lib_ccx/ccx_encoders_webvtt.c has two defects in its --webvtt-create-css path.

1. Header re-emitted before every cue when the CSS file can't be created

https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/ccx_encoders_webvtt.c#L255-L261

FILE *f = fopen(css_file_name, "wb");
if (f == NULL)
{
        mprint("Warning: Error creating the file %s\n", css_file_name);
        free(css_file_name);
        return;                 // <-- skips wrote_webvtt_header = 1
}

The function guards its own re-entry with if (context->wrote_webvtt_header) return; at the top, and sets that flag on the last line:

context->wrote_webvtt_header = 1; // Do it even if couldn't write the header, because it won't be possible anyway

That trailing comment states the intent explicitly: the flag should be set even when the header could not be fully written. The return above bypasses it.

write_webvtt_header() is called from write_stringz_as_webvtt(), write_cc_bitmap_as_webvtt() and write_cc_buffer_as_webvtt() — i.e. once per cue. So when the CSS file cannot be created, the flag never gets set and the whole header block (either the X-TIMESTAMP-MAP line or the blank line, depending on --timestamp-map) is written again before every subsequent cue, in the middle of the file. The resulting .vtt is not valid WebVTT.

Repro:

mkdir ro && cd ro
ccextractor ../any_input.ts --out=webvtt --webvtt-create-css -o out.vtt
# make the CSS creation fail, e.g. by making the CWD read-only first:
#   chmod a-w .        (Linux/macOS)
# then inspect out.vtt - the header block repeats before each cue

(fopen for the .css uses the current working directory, not the output path, so a read-only CWD is enough — which is itself a little surprising.)

2. basefilename is leaked on every path

https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/ccx_encoders_webvtt.c#L247

char *basefilename = get_basename(context->first_input_file);

get_basename() returns malloced memory (utility.c), but basefilename is never freed — not on the success path, not on the fopen-failure path. Every other caller in the tree frees it (ccx_encoders_common.c:607, :636, :1369, :1437).

Normally that is a single small leak per output file. Combined with defect 1 it becomes a leak per cue, since the function then re-runs for the whole recording.

3. Minor: unguarded get_basename() return

get_basename() returns NULL if its argument is NULL or on allocation failure, and strlen(basefilename) on the next line would then crash. ccx_encoders_common.c:636-641 guards exactly this case:

char *basefilename = get_basename(ctx->first_input_file);
...
if (basefilename == NULL)
{
        basefilename = get_basename("untitled");
}

To be straight about this one: I could not establish that first_input_file is ever actually NULL on a live run — the Rust parser leaves it as an empty string rather than NULL (parser.rs:1618-1620), so in practice get_basename() returns "". I'm flagging it as hardening consistent with the existing guard, not as a demonstrated crash. Happy to drop it from the patch if you'd rather keep the diff to the two real defects.

Verification

I confirmed defects 1 and 2 by reading the code and compiling; both are static facts about the control flow (a return that skips an assignment, and an allocation with no matching free) rather than behavioural guesses, so they're checkable in about twenty lines. I did not execute the fopen-failure path, since I don't have a full local build — so I'd appreciate a sanity check on the repro above rather than taking my word for the runtime effect.

PR follows.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions