Skip to content

Fix null pointer dereference in Matroska parser on file open failure#2171

Open
apoorvdarshan wants to merge 1 commit intoCCExtractor:masterfrom
apoorvdarshan:fix/matroska-null-file-pointer
Open

Fix null pointer dereference in Matroska parser on file open failure#2171
apoorvdarshan wants to merge 1 commit intoCCExtractor:masterfrom
apoorvdarshan:fix/matroska-null-file-pointer

Conversation

@apoorvdarshan
Copy link
Contributor

@apoorvdarshan apoorvdarshan commented Mar 4, 2026

Summary

  • create_file() returns the result of fopen() without checking for NULL
  • matroska_loop() passes this directly into matroska_parse(), which calls feof() on the NULL pointer, crashing the program
  • The NULL file pointer propagates to 10+ usage sites throughout the parser
  • Added a NULL check after create_file() that prints an error, frees mkv_ctx, and returns -1

Test plan

  • Build the project and verify no compilation errors
  • Run ccextractor with a nonexistent MKV file path and verify it prints an error instead of crashing

create_file() returns the result of fopen() which can be NULL if the
file cannot be opened. matroska_loop() never checked this, passing
the NULL pointer into matroska_parse() where it is immediately used
in feof(), causing a crash. Add a NULL check and return an error.
Copy link
Contributor

@cfsmp3 cfsmp3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as the feedback on #2157: use fatal(EXIT_READ_ERROR, ...) instead of mprint() + return. If the input file can't be opened, the program should exit with a proper error code, not silently return and appear to succeed. The very next error check in this function (malloc for sub_tracks) uses fatal() — be consistent.

Also remove the CHANGES.TXT entry — this is an internal fix, not a user-reported bug.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants