Skip to content

Commit

Permalink
comps: Fix memory issues in group serialization
Browse files Browse the repository at this point in the history
This commit addresses two memory issues that occur if saving of the
group XML file fails:

Memory leak: The `doc` variable is not properly freed before an error is
thrown, leading to a memory leak.

Use after free: The libxml2 error handler continues to reference the
`error_to_strings()` function, which uses the `xml_errors` vector that
is local to the `Group::serialize()` method. If the handler is invoked
later (e.g., during the serialization of an Environment), it may cause a
crash due to accessing freed memory.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2315789
  • Loading branch information
m-blaha committed Oct 1, 2024
1 parent f27f1d4 commit 12f3bbc
Showing 1 changed file with 8 additions and 6 deletions.
14 changes: 8 additions & 6 deletions libdnf5/comps/group/group.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,14 @@ void Group::serialize(const std::string & path) {
}

// Save the document
if (xmlSaveFormatFileEnc(path.c_str(), doc, "utf-8", 1) == -1) {
auto save_result = xmlSaveFormatFileEnc(path.c_str(), doc, "utf-8", 1);

// Memory free
xmlFreeDoc(doc);
// reset the error handler to default
xmlSetGenericErrorFunc(NULL, NULL);

if (save_result == -1) {
// There can be duplicit messages in the libxml2 errors so make them unique
auto it = unique(xml_errors.begin(), xml_errors.end());
xml_errors.resize(static_cast<size_t>(distance(xml_errors.begin(), it)));
Expand All @@ -333,11 +340,6 @@ void Group::serialize(const std::string & path) {
path,
libdnf5::utils::string::join(xml_errors, ", "));
}

// Memory free
xmlFreeDoc(doc);
// reset the error handler to default
xmlSetGenericErrorFunc(NULL, NULL);
}

libdnf5::transaction::TransactionItemReason Group::get_reason() const {
Expand Down

0 comments on commit 12f3bbc

Please sign in to comment.