fix(dvb_subtitle_decoder): add NULL checks after malloc calls#1794
Merged
fix(dvb_subtitle_decoder): add NULL checks after malloc calls#1794
Conversation
This commit addresses multiple memory safety issues in the Matroska parser identified through static analysis (cppcheck). ## Null pointer dereference after malloc (15 fixes) Added null checks after all malloc/calloc calls to prevent crashes when memory allocation fails: - read_byte_block(): line 28 - read_bytes_signed(): line 38 - generate_timestamp_ass_ssa(): line 267 - parse_segment_cluster_block_group_block(): lines 306, 361 - parse_segment_cluster_block_group_block_additions(): line 405 - parse_segment_cluster_block_group(): line 476 - parse_segment_track_entry(): lines 958, 973 - parse_private_codec_data(): line 1019 - generate_filename_from_track(): line 1167 - ass_ssa_sentence_erase_read_order(): line 1191 - save_sub_track(): lines 1264, 1271, 1303, 1310 - matroska_loop(): lines 1496, 1505 ## Buffer overflow fixes (3 fixes) - generate_timestamp_ass_ssa(): Increased buffer from 15 to 32 bytes, changed sprintf to snprintf. GCC warned output could be 11-23 bytes. - save_sub_track(): Increased number[] buffer from 9 to 16 bytes, changed sprintf to snprintf. - generate_filename_from_track(): Now calculates required buffer size dynamically instead of using fixed 200 bytes. ## Memory leak fixes (7 fixes) - parse_ebml(): Fixed leak of read_vint_block_string() return value - parse_segment_info(): Fixed 4 leaks of read_vint_block_string() returns (filename, title, muxing_app, writing_app) - parse_segment_track_entry(): Added free(lang) before reassignment - save_sub_track(): Fixed leak where text pointer was advanced, losing original allocation ## Realloc error handling (3 fixes) Fixed realloc calls to use temporary variable, preventing loss of original pointer if realloc fails: - parse_segment_cluster_block_group_block(): line 366 - parse_segment_cluster_block_group(): line 475 - parse_segment_track_entry(): line 973 ## Use-after-free fix (1 fix) - matroska_loop(): Saved avc_track_number and dec_sub.got_output before calling matroska_free_all(), then used saved values ## Missing free fixes (2 fixes) - free_sub_track(): Added free(track->sentences) for the array itself - matroska_free_all(): Added free(mkv_ctx->sub_tracks) for the array ## Other improvements - Initialized sub_track->sentences to NULL in parse_segment_track_entry() to ensure safe NULL check in free_sub_track() All changes use EXIT_NOT_ENOUGH_MEMORY (exit code 500) for out-of-memory conditions, consistent with the rest of the codebase. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add missing NULL checks for 9 malloc() calls in the DVB subtitle decoder that could cause crashes or undefined behavior if memory allocation fails. All checks use fatal(EXIT_NOT_ENOUGH_MEMORY, ...) to terminate gracefully with an appropriate error message, consistent with the approach used in matroska.c and other parts of the codebase. Affected functions and allocations: - dvbsub_init_decoder(): DVBSubContext allocation - dvbsub_parse_clut_segment(): DVBSubCLUT allocation - dvbsub_parse_region_segment(): DVBSubRegion, pbuf, DVBSubObject, and DVBSubObjectDisplay allocations - dvbsub_parse_page_segment(): DVBSubRegionDisplay allocation - write_dvb_sub(): cc_bitmap (rect), data1, and data0 allocations - dvbsub_handle_display_segment(): private_data allocation This also fixes a potential memory leak in write_dvb_sub() where rect and rect->data1 would be leaked if the rect->data0 allocation failed (previously returned -1 without cleanup, now terminates via fatal()). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
This PR adds missing NULL checks for 9 malloc() calls in the DVB subtitle decoder (
src/lib_ccx/dvb_subtitle_decoder.c) that could cause crashes or undefined behavior if memory allocation fails.Changes
All checks use
fatal(EXIT_NOT_ENOUGH_MEMORY, ...)to terminate gracefully with an appropriate error message, consistent with the approach used inmatroska.cand other parts of the codebase.Affected Functions and Allocations
dvbsub_init_decoder()DVBSubContextdvbsub_parse_clut_segment()DVBSubCLUTdvbsub_parse_region_segment()DVBSubRegiondvbsub_parse_region_segment()region->pbufdvbsub_parse_region_segment()DVBSubObjectdvbsub_parse_region_segment()DVBSubObjectDisplaydvbsub_parse_page_segment()DVBSubRegionDisplaywrite_dvb_sub()cc_bitmap(rect)write_dvb_sub()rect->data1write_dvb_sub()rect->data0dvbsub_handle_display_segment()private_dataAdditional Fix
This also fixes a potential memory leak in
write_dvb_sub()whererectandrect->data1would be leaked if therect->data0allocation failed (previously returned -1 without cleanup, now terminates viafatal()).Context
This is part of a systematic effort to improve memory safety across the CCExtractor codebase. The DVB subtitle decoder was identified as having 28 memory-related function calls, making it a high-priority target for review.
Test plan
🤖 Generated with Claude Code