fix(lib_ccx): replace unsafe string functions with bounds-checked versions #1807
+466
−150
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rsions Replace sprintf/strcpy/strcat with snprintf/strncat/memmove in: - ccx_encoders_common.c: 4 sprintf -> snprintf - ccx_encoders_helpers.c: 3 strcat -> strncat, 1 strcpy -> memcpy - telxcc.c: 3 sprintf -> snprintf - asf_functions.c: 3 sprintf -> snprintf - ccx_encoders_ssa.c: 3 sprintf -> snprintf - ccx_encoders_curl.c: 1 sprintf -> snprintf, strcpy+strcat -> snprintf with OOM check - ccx_encoders_splitbysentence.c: 1 strcpy -> memmove (overlapping memory fix), 2 strcat -> strncat This is part of Phase 3.1 of the buffer safety audit, addressing MEDIUM priority files. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…hecked versions Replace sprintf/strcpy with snprintf/memcpy in LOW priority files: - general_loop.c: proper buffer allocation with OOM check, snprintf - ccx_encoders_g608.c: snprintf with sizeof for timeline buffer - lib_ccx.c: fix buffer size calculation, add missing null check, snprintf - ccx_common_timing.c: snprintf with documented max size for time functions - ts_functions.c: snprintf with sizeof in debug code - matroska.c: bounded memcpy to prevent overflow from malformed language codes - output.c: snprintf with known allocated size This completes Phase 3.1 of the buffer safety audit. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
cfsmp3
changed the title
Contributor
Author
|
Added LOW priority files to complete Phase 3.1 (Dangerous Function Patterns):
This completes all sprintf/strcpy/strcat replacements in Phase 3.1. |
This commit addresses multiple memory safety issues in ccx_encoders_spupng.c: **NULL pointer dereference fixes (crash prevention):** 1. write_cc_bitmap_as_spupng() line 440: Added NULL check after malloc for pbuf - previously would crash on memset if allocation failed. 2. write_image() line 541: Added NULL check after malloc for row buffer with proper cleanup via goto finalise. 3. center_justify() line 611: Added NULL check after malloc for temp_buffer - previously would crash immediately on use. 4. utf8_to_utf32() line 718: Added NULL check after calloc for string_utf32 - previously would crash on use by iconv. 5. spupng_export_string2png() line 780: Fixed existing NULL check that printed error but did not return/exit - code would continue to memset(NULL, ...) causing a crash. **Memory leak fixes:** 6. spupng_export_string2png() line 789: Fixed leak where buffer was not freed when strdup(str) failed and function returned early. 7. spupng_export_string2png() line 901: Fixed leak on realloc failure where buffer, tmp, and string_utf32 were leaked. Now properly frees all three before calling fatal(). All fatal() calls include diagnostic information (function name and bytes requested where applicable) to aid debugging OOM conditions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- search_language_pack: add NULL check after strdup(), fix unsafe realloc() that lost original pointer on failure - init_ocr: fix memory leak where ctx wasn't freed on early return when tessdata not found, add NULL checks for strdup() calls - ocr_bitmap: fix memory leak when pixCreate partially fails, add missing boxDestroy for crop_points on early return, add NULL checks for histogram/iot/mcit allocations, fix unsafe realloc() calls, add NULL check for text_out strdup - ocr_rect: add NULL check for copy allocation, initialize copy->data to NULL to prevent freep on uninitialized pointer, add NULL check for copy->data allocation - paraof_ocrtext: use fatal() on malloc failure for consistent OOM handling All OOM conditions now use fatal(EXIT_NOT_ENOUGH_MEMORY, ...) following the project's coding patterns. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add NULL checks after malloc calls for compressed_data_buffer and buff_ptr - Replace sprintf with snprintf for all string formatting operations - Replace strcat with bounds-checked direct character assignment - Replace vsprintf with vsnprintf in debug_log function - Replace sprintf loop in random_chars with direct character lookup table - Increase buffer sizes for date_str (50->64), time_str (30->32), tcr_str (25->32) - Initialize tcr_str in default case to prevent uninitialized use - Add lib_ccx.h include for fatal() function declaration Functions modified: - mcc_encode_cc_data: OOM check + sprintf -> snprintf + strcat -> direct assignment - generate_mcc_header: sprintf -> snprintf for uuid_str, date_str, time_str, tcr_str - add_boilerplate: OOM check for buff_ptr - random_chars: sprintf -> direct character lookup (more efficient) - debug_log: vsprintf -> vsnprintf + safer strlen check 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace all sprintf calls with snprintf to prevent potential buffer overflows in CEA-708 output functions. Key changes: - dtvcc_change_pen_colors: add bounds checking for font color tags - dtvcc_change_pen_attribs: add bounds checking for italic/underline tags - dtvcc_write_srt: track buffer length with snprintf - dtvcc_write_transcript: add bounds checking for CC/mode labels - dtvcc_write_sami_header: use snprintf macro for all SAMI tags - dtvcc_write_sami_footer: use snprintf with length check - dtvcc_write_sami: add bounds checking for sync tags - dtvcc_write_scc_header: use snprintf for SCC header - add_needed_scc_labels: add buffer size parameter for safe writes - dtvcc_write_scc: use snprintf macro for all SCC formatting - dtvcc_writer_init: use snprintf for filename suffix 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fix macro formatting to have 'do' and '{' on separate lines and
align backslashes consistently, as required by clang-format.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- EPG_output_live: add NULL checks for filename/finalfilename malloc, add fopen failure check - EPG_DVB_decode_string: add NULL checks for decode_buffer and out malloc - EPG_decode_content_descriptor: add NULL check for categories malloc - EPG_decode_parental_rating_descriptor: add NULL check for ratings malloc - EPG_decode_extended_event_descriptor: add NULL checks for net and extended_text malloc - EPG_ATSC_decode_multiple_string: add NULL checks for event_name and text malloc - parse_EPG_packet: add NULL check for buffer malloc, fix unsafe realloc that lost original pointer on failure - EPG_decode_short_event_descriptor: fix memory leak - free event_name on early return - EPG_DVB_decode_EIT: fix memory leak - call EPG_free_event on early return All OOM conditions now use fatal(EXIT_NOT_ENOUGH_MEMORY, ...) following the project's coding patterns. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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 249cac3...:
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:
All tests passing on the master branch were passed completely. Check the result page for more info. |
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 1510396...:
Your PR breaks these cases:
Congratulations: Merging this PR would fix the following tests:
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Replace sprintf/strcpy/strcat with bounds-checked versions in files identified in Phase 3.1 of the buffer safety audit. This completes all dangerous function pattern fixes.
Files Modified
Medium Priority (previous commit)
Low Priority (latest commit)
Notable Fixes
ccx_encoders_splitbysentence.c: Fixed undefined behavior where
strcpy()was used with overlapping memory regions. Replaced withmemmove().ccx_encoders_curl.c: Added OOM check for malloc allocation that was previously unchecked.
lib_ccx.c: Fixed buffer size calculation - was allocating
strlen + 10but neededstrlen + 12for the format string.matroska.c: Protected against malformed language codes that could cause buffer overflow from untrusted file input.
Test plan
🤖 Generated with Claude Code