Fix double-free in test::ParseMkvFile cleanup logic #7
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 fixes a double-free bug in libwebm’s test utility helper where
test::ParseMkvFile()could free the same heap objects twice, corrupting the heap. The fix enforces single ownership of parser resources and relies on RAII to guarantee safe cleanup on all paths.Severity
S1 – double-free leading to heap corruption.
Affected Components
testing/test_util.cctest::ParseMkvFile()test::ParseMkvFileReleaseParser()test::MkvParser::~MkvParser()Vulnerability Description
test::ParseMkvFile()constructed a stack-allocatedMkvParserobject whose destructor deletes its ownedsegmentandreadermembers. The function also manually deletedparser.segmentandparser.readerbefore returning. When the function scope ended,MkvParser::~MkvParser()ran and freed the same objects again, resulting in a double-free.Certain failure paths could still allocate
reader, making the double-free reachable even when parsing fails.Fix Description
deletecalls intest::ParseMkvFile()and relied exclusively onMkvParser’s destructor for cleanup.test::ParseMkvFileReleaseParser()to usestd::unique_ptrfor temporary ownership and to transfer raw pointers to the output parser only after successful initialization, preventing partial-state ownership confusion.Before vs After
Before:
segmentandreadercould be freed twice on function return, corrupting the heap.After:
Parser resources have a single clear owner and are freed exactly once via RAII, even on failure paths.
Testing
test::ParseMkvFile().