Skip to content

Fix unchecked fopen() and alloc_demuxer_data() return values in proce…#2211

Open
NexionisJake wants to merge 1 commit intoCCExtractor:masterfrom
NexionisJake:fix/null-checks-process-hex
Open

Fix unchecked fopen() and alloc_demuxer_data() return values in proce…#2211
NexionisJake wants to merge 1 commit intoCCExtractor:masterfrom
NexionisJake:fix/null-checks-process-hex

Conversation

@NexionisJake
Copy link

[FIX] Fix unchecked fopen() and alloc_demuxer_data() return
values in process_hex()

  • Add NULL check for fopen() return; call fatal() with appropriate
    error code if the file cannot be opened, preventing a segfault on
    fgets().
  • Add NULL check for alloc_demuxer_data() return; close the
    already-opened file handle before calling fatal() to avoid a resource
    leak.

Fixes #2201

In raising this pull request, I confirm the following :

Reason for this PR:

  • This PR adds new functionality.
  • This PR fixes a bug that I have personally experienced or that a
    real user has reported and for which a sample exists.
  • This PR is porting code from C to Rust.

Sanity check:

  • I have read and understood the [contributors guide](https://github
    .com/CCExtractor/ccextractor/blob/master/.github/CONTRIBUTING.md).
  • I have checked that another pull request for this purpose does not
    exist.
  • If the PR adds new functionality, I've added it to the changelog.
    If it's just a bug fix, I have NOT added it to the changelog.
  • I am NOT adding new C code unless it's to fix an existing,
    reproducible bug.

Repro instructions:

process_hex() in src/lib_ccx/general_loop.c is reachable when
CCExtractor is built with -DWTV_DEBUG and a .hex dump file is passed
as input.

Crash 1 — NULL fopen() (segfault on fgets):

  1. Build with -DWTV_DEBUG.
  2. Invoke process_hex() with a filename that does not exist or is not
    readable.
  3. Without the fix: fr is NULL, and the immediately following
    fgets(line, max-1, fr) dereferences it → segfault.
  4. With the fix: fatal(CCX_COMMON_EXIT_FILE_CREATION_FAILED, ...) is
    called with a clear error message before any dereference occurs.

Crash 2 — NULL alloc_demuxer_data() (NULL dereference + fd leak):

  1. Build with -DWTV_DEBUG.
  2. Trigger an OOM condition (or mock alloc_demuxer_data() to return
    NULL).
  3. Without the fix: data is NULL, the loop body writes into
    data->buffer → segfault; fr is also leaked.
  4. With the fix: fr is closed first, then
    fatal(EXIT_NOT_ENOUGH_MEMORY, ...) is called cleanly.

The fix is two surgical NULL checks matching the existing error-handling
style used throughout the file (EXIT_NOT_ENOUGH_MEMORY) and the rest
of the project (CCX_COMMON_EXIT_FILE_CREATION_FAILED — same pattern as
ccx_encoders_spupng.c:1137). No other lines were changed.

Note: PR #2202 also addresses this issue but omits the fclose(fr)
before the second fatal() call, leaving a file descriptor leak. This
PR includes that cleanup.

…ss_hex()

- Add NULL check for fopen() return; call fatal() with appropriate error
  code if the file cannot be opened, preventing a segfault on fgets().
- Add NULL check for alloc_demuxer_data() return; close the already-opened
  file handle before calling fatal() to avoid a resource leak.

Fixes CCExtractor#2201
@ccextractor-bot
Copy link
Collaborator

CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results, when compared to test for commit 03ad9e8...:
Report Name Tests Passed
Broken 10/13
CEA-708 1/14
DVB 4/7
DVD 3/3
DVR-MS 2/2
General 27/27
Hardsubx 1/1
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 79/86
Teletext 20/21
WTV 13/13
XDS 34/34

Your PR breaks these cases:

  • ccextractor --startcreditsnotafter 2 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9...
  • ccextractor --startcreditsforatmost 2 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9...

NOTE: The following tests have been failing on the master branch as well as the PR:

Congratulations: Merging this PR would fix the following tests:

  • ccextractor --autoprogram --out=ttxt --latin1 --ucla --xds 8e8229b88b..., Last passed: Never
  • ccextractor --autoprogram --out=srt --latin1 --quant 0 85271be4d2..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 132d7df7e9..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 99e5eaafdc..., Last passed: Never
  • ccextractor --autoprogram --out=srt --latin1 b22260d065..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 --ucla 7aad20907e..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 --ucla dab1c1bd65..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 01509e4d27..., Last passed: Never
  • ccextractor --out=srt --latin1 --autoprogram 29e5ffd34b..., Last passed: Never
  • ccextractor --out=spupng c83f765c66..., Last passed: Never
  • ccextractor --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed: Never
  • ccextractor --startcreditsnotbefore 1 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed: Never
  • ccextractor --startcreditsforatleast 1 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --xds --latin1 --ucla 85058ad37e..., Last passed: Never
  • ccextractor --autoprogram --out=srt --latin1 --ucla b22260d065..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 --ucla --xds 7f41299cc7..., Last passed: Never

It seems that not all tests were passed completely. This is an indication that the output of some files is not as expected (but might be according to you).

Check the result page for more info.

@ccextractor-bot
Copy link
Collaborator

CCExtractor CI platform finished running the test files on windows. Below is a summary of the test results, when compared to test for commit 03ad9e8...:
Report Name Tests Passed
Broken 10/13
CEA-708 1/14
DVB 3/7
DVD 3/3
DVR-MS 2/2
General 25/27
Hardsubx 1/1
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 75/86
Teletext 20/21
WTV 13/13
XDS 34/34

Your PR breaks these cases:

  • ccextractor --autoprogram --out=srt --latin1 --quant 0 85271be4d2...
  • ccextractor --autoprogram --out=ttxt --latin1 --ucla dab1c1bd65...
  • ccextractor --out=srt --latin1 --autoprogram 29e5ffd34b...
  • ccextractor --out=spupng c83f765c66...
  • ccextractor --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9...
  • ccextractor --startcreditsnotbefore 1 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9...
  • ccextractor --startcreditsnotafter 2 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9...
  • ccextractor --startcreditsforatleast 1 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9...
  • ccextractor --startcreditsforatmost 2 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9...

NOTE: The following tests have been failing on the master branch as well as the PR:

Congratulations: Merging this PR would fix the following tests:

  • ccextractor --autoprogram --out=ttxt --latin1 --ucla --xds 8e8229b88b..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 132d7df7e9..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 99e5eaafdc..., Last passed: Never
  • ccextractor --autoprogram --out=srt --latin1 b22260d065..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 --ucla 7aad20907e..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 01509e4d27..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --xds --latin1 --ucla 85058ad37e..., Last passed: Never
  • ccextractor --autoprogram --out=srt --latin1 --ucla b22260d065..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 --ucla --xds 7f41299cc7..., Last passed: Never

It seems that not all tests were passed completely. This is an indication that the output of some files is not as expected (but might be according to you).

Check the result page for more info.

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.

[BUG] Unchecked fopen() and alloc_demuxer_data() return values in process_hex() in general_loop.c

2 participants