[PR #1802] [MERGED] fix(utility): prevent buffer overruns and add OOM checks in change_filename #2537

New Issue

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

claunia commented

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

📋 Pull Request Information

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

Base: master ← Head: fix/utility-buffer-overruns

📝 Commits (6)

a66fb8c fix(utility): prevent buffer overruns and add OOM checks in change_filename
a2cb65f fix(ccx_encoders_spupng): add NULL checks and fix memory leaks
9841235 fix(ocr): add NULL checks and fix memory leaks
54c7dfa fix(mcc_encoder): prevent buffer overruns and add OOM checks
bcf7eb2 fix(708_output): replace sprintf with snprintf for buffer safety
468bd2c style: fix clang-format issues in macro definitions

📊 Changes

6 files changed (+350 additions, -115 deletions)

View changed files

📝 src/lib_ccx/ccx_decoders_708_output.c (+170 -64)
📝 src/lib_ccx/ccx_decoders_708_output.h (+1 -1)
📝 src/lib_ccx/ccx_encoders_mcc.c (+40 -21)
📝 src/lib_ccx/ccx_encoders_spupng.c (+28 -5)
📝 src/lib_ccx/ocr.c (+83 -9)
📝 src/lib_ccx/utility.c (+28 -15)

📄 Description

Summary

This PR fixes multiple buffer overrun vulnerabilities and missing out-of-memory (OOM) checks in the change_filename() function in src/lib_ccx/utility.c. This function handles file rotation/renaming for output files.

Issues Found and Fixed

1. Missing OOM Checks (3 instances)

The function had three malloc() calls without NULL checks, which could lead to null pointer dereferences if memory allocation failed:

Variable	Line	Risk
`temp_encoder`	323	Immediate dereference on next line
`current_name`	337	Used in `strcpy` immediately after
`newname`	357	Used in `snprintf` immediately after

Fix: Added fatal(EXIT_NOT_ENOUGH_MEMORY, ...) checks after each allocation, following the project's established pattern.

2. Buffer Overflow in `new_extension` (CRITICAL)

// BEFORE: Buffer too small for large iteration numbers
char new_extension[6];
sprintf(new_extension, ".%d", iter);

The buffer could only hold 5 characters plus null terminator (e.g., ".9999"), but iter could theoretically exceed 9999, causing ".10000" (7 chars) to overflow.

Fix: Increased buffer to 16 bytes and use snprintf():

char new_extension[16];
snprintf(new_extension, sizeof(new_extension), ".%d", iter);

3. Unsafe `sprintf()` Usage (4 instances)

All sprintf() calls were replaced with snprintf() with proper size limits:

Original	Fixed
`sprintf(str_number, "%d", ...)`	Eliminated - combined into single `snprintf()`
`sprintf(new_extension, ".%d", iter)`	`snprintf(new_extension, sizeof(new_extension), ".%d", iter)`

4. Unsafe `strcpy()`/`strcat()` Chains

The original code used multiple unsafe string operations:

// BEFORE: No bounds checking
strcpy(current_name, enc_ctx->out->filename);
strcat(current_name, ".");
sprintf(str_number, "%d", enc_ctx->out->renaming_extension);
strcat(current_name, str_number);

Fix: Replaced with bounded operations:

// AFTER: Proper bounds checking
strncpy(current_name, enc_ctx->out->filename, current_name_size - 1);
current_name[current_name_size - 1] = '\0';
// ...
size_t cur_len = strlen(current_name);
snprintf(current_name + cur_len, current_name_size - cur_len, ".%d", enc_ctx->out->renaming_extension);

5. Insufficient Buffer Padding

// BEFORE: Only 10 bytes padding - could overflow with extensions like ".12345"
char *current_name = malloc(sizeof(char) * (strlen(enc_ctx->out->filename) + 10));

Fix: Increased to 16 bytes padding and track sizes properly:

size_t filename_len = strlen(enc_ctx->out->filename);
size_t current_name_size = filename_len + 16;
char *current_name = malloc(current_name_size);

Code Changes Summary

Metric	Before	After
`malloc()` without NULL check	3	0
`sprintf()` calls	4	0
`strcpy()` calls	3	0
`strcat()` calls	5	0
`snprintf()` calls	0	5
`strncpy()` calls	0	2
Buffer size for extensions	6 bytes	16 bytes
Buffer padding for filenames	10 bytes	16 bytes

Security Impact

These fixes prevent:

Null pointer dereference crashes when memory is exhausted
Stack buffer overflow via new_extension[6] with large iteration counts
Heap buffer overflow via current_name and newname with long filenames or many rotations

While exploitation would require specific conditions (memory pressure or many file rotations), these are defense-in-depth improvements for a tool that processes untrusted media files.

Test Plan

Code compiles without warnings
Build completes successfully
Manual testing of file rotation feature (if available)
Verify OOM handling terminates gracefully with error message

🤖 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/1802 **Author:** [@cfsmp3](https://github.com/cfsmp3) **Created:** 12/12/2025 **Status:** ✅ Merged **Merged:** 12/13/2025 **Merged by:** [@cfsmp3](https://github.com/cfsmp3) **Base:** `master` ← **Head:** `fix/utility-buffer-overruns` --- ### 📝 Commits (6) - [`a66fb8c`](https://github.com/CCExtractor/ccextractor/commit/a66fb8c6612b448824f9f2f975f1a56cd5ea215a) fix(utility): prevent buffer overruns and add OOM checks in change_filename - [`a2cb65f`](https://github.com/CCExtractor/ccextractor/commit/a2cb65f18121710fd48529d8aa90ddcd3cf0fdb9) fix(ccx_encoders_spupng): add NULL checks and fix memory leaks - [`9841235`](https://github.com/CCExtractor/ccextractor/commit/984123521dded25b1a05489b98be92dc8da415f7) fix(ocr): add NULL checks and fix memory leaks - [`54c7dfa`](https://github.com/CCExtractor/ccextractor/commit/54c7dfa45fb850acb72597365951170a16059f14) fix(mcc_encoder): prevent buffer overruns and add OOM checks - [`bcf7eb2`](https://github.com/CCExtractor/ccextractor/commit/bcf7eb2a5048d317508da30c19041469c1a7467e) fix(708_output): replace sprintf with snprintf for buffer safety - [`468bd2c`](https://github.com/CCExtractor/ccextractor/commit/468bd2c1567bd89daa6a3f78c8f50741d08d76a3) style: fix clang-format issues in macro definitions ### 📊 Changes **6 files changed** (+350 additions, -115 deletions) <details> <summary>View changed files</summary> 📝 `src/lib_ccx/ccx_decoders_708_output.c` (+170 -64) 📝 `src/lib_ccx/ccx_decoders_708_output.h` (+1 -1) 📝 `src/lib_ccx/ccx_encoders_mcc.c` (+40 -21) 📝 `src/lib_ccx/ccx_encoders_spupng.c` (+28 -5) 📝 `src/lib_ccx/ocr.c` (+83 -9) 📝 `src/lib_ccx/utility.c` (+28 -15) </details> ### 📄 Description ## Summary This PR fixes multiple buffer overrun vulnerabilities and missing out-of-memory (OOM) checks in the `change_filename()` function in `src/lib_ccx/utility.c`. This function handles file rotation/renaming for output files. ## Issues Found and Fixed ### 1. Missing OOM Checks (3 instances) The function had three `malloc()` calls without NULL checks, which could lead to null pointer dereferences if memory allocation failed: | Variable | Line | Risk | |----------|------|------| | `temp_encoder` | 323 | Immediate dereference on next line | | `current_name` | 337 | Used in `strcpy` immediately after | | `newname` | 357 | Used in `snprintf` immediately after | **Fix:** Added `fatal(EXIT_NOT_ENOUGH_MEMORY, ...)` checks after each allocation, following the project's established pattern. ### 2. Buffer Overflow in `new_extension` (CRITICAL) ```c // BEFORE: Buffer too small for large iteration numbers char new_extension[6]; sprintf(new_extension, ".%d", iter); ``` The buffer could only hold 5 characters plus null terminator (e.g., ".9999"), but `iter` could theoretically exceed 9999, causing ".10000" (7 chars) to overflow. **Fix:** Increased buffer to 16 bytes and use `snprintf()`: ```c char new_extension[16]; snprintf(new_extension, sizeof(new_extension), ".%d", iter); ``` ### 3. Unsafe `sprintf()` Usage (4 instances) All `sprintf()` calls were replaced with `snprintf()` with proper size limits: | Original | Fixed | |----------|-------| | `sprintf(str_number, "%d", ...)` | Eliminated - combined into single `snprintf()` | | `sprintf(new_extension, ".%d", iter)` | `snprintf(new_extension, sizeof(new_extension), ".%d", iter)` | ### 4. Unsafe `strcpy()`/`strcat()` Chains The original code used multiple unsafe string operations: ```c // BEFORE: No bounds checking strcpy(current_name, enc_ctx->out->filename); strcat(current_name, "."); sprintf(str_number, "%d", enc_ctx->out->renaming_extension); strcat(current_name, str_number); ``` **Fix:** Replaced with bounded operations: ```c // AFTER: Proper bounds checking strncpy(current_name, enc_ctx->out->filename, current_name_size - 1); current_name[current_name_size - 1] = '\0'; // ... size_t cur_len = strlen(current_name); snprintf(current_name + cur_len, current_name_size - cur_len, ".%d", enc_ctx->out->renaming_extension); ``` ### 5. Insufficient Buffer Padding ```c // BEFORE: Only 10 bytes padding - could overflow with extensions like ".12345" char *current_name = malloc(sizeof(char) * (strlen(enc_ctx->out->filename) + 10)); ``` **Fix:** Increased to 16 bytes padding and track sizes properly: ```c size_t filename_len = strlen(enc_ctx->out->filename); size_t current_name_size = filename_len + 16; char *current_name = malloc(current_name_size); ``` ## Code Changes Summary | Metric | Before | After | |--------|--------|-------| | `malloc()` without NULL check | 3 | 0 | | `sprintf()` calls | 4 | 0 | | `strcpy()` calls | 3 | 0 | | `strcat()` calls | 5 | 0 | | `snprintf()` calls | 0 | 5 | | `strncpy()` calls | 0 | 2 | | Buffer size for extensions | 6 bytes | 16 bytes | | Buffer padding for filenames | 10 bytes | 16 bytes | ## Security Impact These fixes prevent: - **Null pointer dereference** crashes when memory is exhausted - **Stack buffer overflow** via `new_extension[6]` with large iteration counts - **Heap buffer overflow** via `current_name` and `newname` with long filenames or many rotations While exploitation would require specific conditions (memory pressure or many file rotations), these are defense-in-depth improvements for a tool that processes untrusted media files. ## Test Plan - [x] Code compiles without warnings - [x] Build completes successfully - [ ] Manual testing of file rotation feature (if available) - [ ] Verify OOM handling terminates gracefully with error message 🤖 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:40 +00:00

claunia closed this issue

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

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/ccextractor#2537

[PR #1802] [MERGED] fix(utility): prevent buffer overruns and add OOM checks in change_filename #2537