Fixing a use after free when trying to load a marker in a category that has attributes defined in another file

Author: AsherGlick

xml_converter/src/packaging_xml.cpp

@@ -239,6 +239,23 @@ static vector<Parseable*> parse_pois(rapidxml::xml_node<>* root_node, map<string
     return markers;
 }
 
+class XMLFileData {
+ public:
+    unique_ptr<basic_istream<char>> raw_file;
+    rapidxml::file<char>* xml_file;
+    string display_path;
+    rapidxml::xml_document<char> xml_document;
+
+    XMLFileData(unique_ptr<basic_istream<char>> input, const MarkerPackFile& filepath) {
+        this->raw_file = std::move(input);
+        this->xml_file = new rapidxml::file<char>(*this->raw_file);
+        this->display_path = filepath.tmp_get_path();
+        this->xml_document.parse<rapidxml::parse_non_destructive | rapidxml::parse_no_data_nodes>(this->xml_file->data(), this->display_path.c_str());
+    }
+};
+
+vector<XMLFileData*> xml_files_to_cleanup;
+
 ////////////////////////////////////////////////////////////////////////////////
 // parse_xml_file
 //
@@ -251,16 +268,10 @@ set<string> parse_xml_file(
 ) {
     vector<XMLError*> errors;
 
-    unique_ptr<basic_istream<char>> infile = open_file_for_read(xml_filepath);
-
-    rapidxml::file<> xml_file(*infile);
-
-    string tmp_get_path = xml_filepath.tmp_get_path();
+    XMLFileData* xml_file_data = new XMLFileData(open_file_for_read(xml_filepath), xml_filepath); // This is a memory leak
+    xml_files_to_cleanup.push_back(xml_file_data);
 
-    rapidxml::xml_document<> doc;
-    doc.parse<rapidxml::parse_non_destructive | rapidxml::parse_no_data_nodes>(xml_file.data(), tmp_get_path.c_str());
-
-    rapidxml::xml_node<>* root_node = doc.first_node();
+    rapidxml::xml_node<>* root_node = xml_file_data->xml_document.first_node();
     // Validate the Root Node
     if (get_node_name(root_node) != "OverlayData") {
         errors.push_back(new XMLNodeNameError("Root node should be of type OverlayData", root_node));
@@ -295,6 +306,13 @@ set<string> parse_xml_file(
     return category_names;
 }
 
+void cleanup_xml_files() {
+    for (size_t i = 0; i < xml_files_to_cleanup.size(); i++) {
+        delete xml_files_to_cleanup[i]->xml_file;
+        delete xml_files_to_cleanup[i];
+    }
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 ////////////////////////////////// SERIALIZE ///////////////////////////////////
 ////////////////////////////////////////////////////////////////////////////////

xml_converter/src/packaging_xml.hpp

@@ -21,3 +21,5 @@ void write_xml_file(
     std::map<std::string, Category>* marker_categories,
     std::vector<Parseable*>* parsed_pois
 );
+
+void cleanup_xml_files();

xml_converter/src/xml_converter.cpp

@@ -53,6 +53,7 @@ map<string, vector<string>> read_taco_directory(
         }
     }
 
+    cleanup_xml_files();
     return top_level_category_file_locations;
 }