[PR #1804] [MERGED] fix(mcc_encoder): prevent buffer overruns and add OOM checks #2541

New Issue

claunia · 2026-01-29T17:22:41Z

claunia commented

2026-01-29 17:22:41 +00:00

📋 Pull Request Information

Original PR: https://github.com/CCExtractor/ccextractor/pull/1804
Author: @cfsmp3
Created: 12/13/2025
Status: ✅ Merged
Merged: 12/13/2025
Merged by: @cfsmp3

Base: master ← Head: fix/mcc-encoder-buffer-overruns

📝 Commits (1)

37fed5e fix(mcc_encoder): prevent buffer overruns and add OOM checks

📊 Changes

1 file changed (+40 additions, -21 deletions)

View changed files

📝 src/lib_ccx/ccx_encoders_mcc.c (+40 -21)

📄 Description

Summary

This PR fixes multiple buffer overrun vulnerabilities and missing out-of-memory (OOM) checks in the MCC (MacCaption) encoder in src/lib_ccx/ccx_encoders_mcc.c.

Issues Found and Fixed

1. Missing OOM Checks (2 instances)

Function	Variable	Risk
`mcc_encode_cc_data`	`compressed_data_buffer`	NULL dereference in sprintf/strcat
`add_boilerplate`	`buff_ptr`	NULL dereference in array assignments

Fix: Added fatal(EXIT_NOT_ENOUGH_MEMORY, ...) checks after each allocation.

2. Unsafe `sprintf` Calls (12 instances)

All sprintf calls into fixed-size buffers were replaced with snprintf:

Function	Buffer	Original Size	New Size
`mcc_encode_cc_data`	`compressed_data_buffer`	dynamic	dynamic (with size tracking)
`generate_mcc_header`	`uuid_str`	50	50
`generate_mcc_header`	`date_str`	50	64
`generate_mcc_header`	`time_str`	30	32
`generate_mcc_header`	`tcr_str` (7 locations)	25	32

3. Unsafe `strcat` Call

Before:

strcat(compressed_data_buffer, "\n");

After:

size_t current_len = strlen(compressed_data_buffer);
if (current_len + 1 < compressed_data_size)
{
    compressed_data_buffer[current_len] = '\n';
    compressed_data_buffer[current_len + 1] = '\0';
}

4. Unsafe `vsprintf` in `debug_log`

Before:

char message[1024];
vsprintf(message, fmt, args);

After:

char message[1024];
vsnprintf(message, sizeof(message), fmt, args);

5. Inefficient `sprintf` Loop in `random_chars`

Before:

for (int i = 0; i < len; i++)
{
    sprintf(buffer + i, "%X", rand() % 16);  // Writes 2 bytes per iteration (char + null)
}

After:

static const char hex_chars[] = "0123456789ABCDEF";
for (int i = 0; i < len; i++)
{
    buffer[i] = hex_chars[rand() % 16];  // Direct assignment, more efficient
}

6. Uninitialized Buffer in Default Case

Added tcr_str[0] = '\0'; in the default case of the framerate switch to prevent using uninitialized data.

Code Changes Summary

Metric	Before	After
`malloc()` without NULL check	2	0
`sprintf()` calls	12	0
`snprintf()` calls	0	12
`strcat()` calls	1	0
`vsprintf()` calls	1	0
`vsnprintf()` calls	0	1

Security Impact

These fixes prevent:

Null pointer dereference crashes when memory is exhausted
Stack buffer overflow via fixed-size string buffers
Heap buffer overflow via dynamically allocated buffers
Use of uninitialized data in edge cases

MCC output is used for professional broadcast captioning, making reliability important.

Test Plan

Code compiles without warnings from ccx_encoders_mcc.c
Build completes successfully
Test MCC output generation with sample files
Verify header format is correct (UUID, date, time code rate)

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/CCExtractor/ccextractor/pull/1804 **Author:** [@cfsmp3](https://github.com/cfsmp3) **Created:** 12/13/2025 **Status:** ✅ Merged **Merged:** 12/13/2025 **Merged by:** [@cfsmp3](https://github.com/cfsmp3) **Base:** `master` ← **Head:** `fix/mcc-encoder-buffer-overruns` --- ### 📝 Commits (1) - [`37fed5e`](https://github.com/CCExtractor/ccextractor/commit/37fed5e5b597665e606bcccd7d9fab2fc84788b5) fix(mcc_encoder): prevent buffer overruns and add OOM checks ### 📊 Changes **1 file changed** (+40 additions, -21 deletions) <details> <summary>View changed files</summary> 📝 `src/lib_ccx/ccx_encoders_mcc.c` (+40 -21) </details> ### 📄 Description ## Summary This PR fixes multiple buffer overrun vulnerabilities and missing out-of-memory (OOM) checks in the MCC (MacCaption) encoder in `src/lib_ccx/ccx_encoders_mcc.c`. ## Issues Found and Fixed ### 1. Missing OOM Checks (2 instances) | Function | Variable | Risk | |----------|----------|------| | `mcc_encode_cc_data` | `compressed_data_buffer` | NULL dereference in sprintf/strcat | | `add_boilerplate` | `buff_ptr` | NULL dereference in array assignments | **Fix:** Added `fatal(EXIT_NOT_ENOUGH_MEMORY, ...)` checks after each allocation. ### 2. Unsafe `sprintf` Calls (12 instances) All `sprintf` calls into fixed-size buffers were replaced with `snprintf`: | Function | Buffer | Original Size | New Size | |----------|--------|---------------|----------| | `mcc_encode_cc_data` | `compressed_data_buffer` | dynamic | dynamic (with size tracking) | | `generate_mcc_header` | `uuid_str` | 50 | 50 | | `generate_mcc_header` | `date_str` | 50 | 64 | | `generate_mcc_header` | `time_str` | 30 | 32 | | `generate_mcc_header` | `tcr_str` (7 locations) | 25 | 32 | ### 3. Unsafe `strcat` Call **Before:** ```c strcat(compressed_data_buffer, "\n"); ``` **After:** ```c size_t current_len = strlen(compressed_data_buffer); if (current_len + 1 < compressed_data_size) { compressed_data_buffer[current_len] = '\n'; compressed_data_buffer[current_len + 1] = '\0'; } ``` ### 4. Unsafe `vsprintf` in `debug_log` **Before:** ```c char message[1024]; vsprintf(message, fmt, args); ``` **After:** ```c char message[1024]; vsnprintf(message, sizeof(message), fmt, args); ``` ### 5. Inefficient `sprintf` Loop in `random_chars` **Before:** ```c for (int i = 0; i < len; i++) { sprintf(buffer + i, "%X", rand() % 16); // Writes 2 bytes per iteration (char + null) } ``` **After:** ```c static const char hex_chars[] = "0123456789ABCDEF"; for (int i = 0; i < len; i++) { buffer[i] = hex_chars[rand() % 16]; // Direct assignment, more efficient } ``` ### 6. Uninitialized Buffer in Default Case Added `tcr_str[0] = '\0';` in the default case of the framerate switch to prevent using uninitialized data. ## Code Changes Summary | Metric | Before | After | |--------|--------|-------| | `malloc()` without NULL check | 2 | 0 | | `sprintf()` calls | 12 | 0 | | `snprintf()` calls | 0 | 12 | | `strcat()` calls | 1 | 0 | | `vsprintf()` calls | 1 | 0 | | `vsnprintf()` calls | 0 | 1 | ## Security Impact These fixes prevent: - **Null pointer dereference** crashes when memory is exhausted - **Stack buffer overflow** via fixed-size string buffers - **Heap buffer overflow** via dynamically allocated buffers - **Use of uninitialized data** in edge cases MCC output is used for professional broadcast captioning, making reliability important. ## Test Plan - [x] Code compiles without warnings from ccx_encoders_mcc.c - [x] Build completes successfully - [ ] Test MCC output generation with sample files - [ ] Verify header format is correct (UUID, date, time code rate) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

claunia added the pull-request label 2026-01-29 17:22:41 +00:00

claunia closed this issue

2026-01-29 17:22:41 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/ccextractor#2541

[PR #1804] [MERGED] fix(mcc_encoder): prevent buffer overruns and add OOM checks #2541

📋 Pull Request Information

📝 Commits (1)

📊 Changes

📄 Description

Summary

Issues Found and Fixed

1. Missing OOM Checks (2 instances)

2. Unsafe sprintf Calls (12 instances)

3. Unsafe strcat Call

4. Unsafe vsprintf in debug_log

5. Inefficient sprintf Loop in random_chars

6. Uninitialized Buffer in Default Case

Code Changes Summary

Security Impact

Test Plan

2. Unsafe `sprintf` Calls (12 instances)

3. Unsafe `strcat` Call

4. Unsafe `vsprintf` in `debug_log`

5. Inefficient `sprintf` Loop in `random_chars`