export: Fix use-after-free on multiple calls overwriting same FILE

CMake 3.16 and below allow multiple `export()` calls with the same output
file even without using `APPEND`.  The implementation worked by accident
by leaking memory.  Refactoring in commit 5444a8095d (cmGlobalGenerator:
modernize memrory managemenbt, 2019-12-29, v3.17.0-rc1~239^2) cleaned up
that memory leak and converted it to a use-after-free instead.

The problem is caused by using the `cmGlobalGenerator::BuildExportSets`
map to own `cmExportBuildFileGenerator` instances.  It can own only
one instance per output FILE name at a time, so repeating use of the
same file now frees the old `cmExportBuildFileGenerator` instance
and leaves the pointer in the `cmMakefile::ExportBuildFileGenerators`
vector dangling.  Move ownership of the instances into `cmMakefile`'s
vector since its entries are not replaced on a repeat output FILE.

In future work we should introduce a policy to error out on this case.
For now simply fix the use-after-free to restore CMake <= 3.16 behavior.

Fixes: #20469

1 files changed, 9 insertions, 10 deletions

diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx
index 0404715..6a2d4c7 100644
--- a/Source/cmGlobalGenerator.cxx
+++ b/Source/cmGlobalGenerator.cxx
@@ -262,17 +262,16 @@ void cmGlobalGenerator::ResolveLanguageCompiler(const std::string& lang,
   }
 }
 
-void cmGlobalGenerator::AddBuildExportSet(
-  std::unique_ptr<cmExportBuildFileGenerator> gen)
+void cmGlobalGenerator::AddBuildExportSet(cmExportBuildFileGenerator* gen)
 {
-  this->BuildExportSets[gen->GetMainExportFileName()] = std::move(gen);
+  this->BuildExportSets[gen->GetMainExportFileName()] = gen;
 }
 
 void cmGlobalGenerator::AddBuildExportExportSet(
-  std::unique_ptr<cmExportBuildFileGenerator> gen)
+  cmExportBuildFileGenerator* gen)
 {
-  this->BuildExportExportSets[gen->GetMainExportFileName()] = gen.get();
-  this->AddBuildExportSet(std::move(gen));
+  this->BuildExportExportSets[gen->GetMainExportFileName()] = gen;
+  this->AddBuildExportSet(gen);
 }
 
 bool cmGlobalGenerator::GenerateImportFile(const std::string& file)
@@ -283,7 +282,7 @@ bool cmGlobalGenerator::GenerateImportFile(const std::string& file)
 
     if (!this->ConfigureDoneCMP0026AndCMP0024) {
       for (const auto& m : this->Makefiles) {
-        m->RemoveExportBuildFileGeneratorCMP0024(it->second.get());
+        m->RemoveExportBuildFileGeneratorCMP0024(it->second);
       }
     }
 
@@ -1317,7 +1316,7 @@ cmExportBuildFileGenerator* cmGlobalGenerator::GetExportedTargetsFile(
   const std::string& filename) const
 {
   auto const it = this->BuildExportSets.find(filename);
-  return it == this->BuildExportSets.end() ? nullptr : it->second.get();
+  return it == this->BuildExportSets.end() ? nullptr : it->second;
 }
 
 void cmGlobalGenerator::AddCMP0042WarnTarget(const std::string& target)
@@ -1353,9 +1352,9 @@ bool cmGlobalGenerator::CheckALLOW_DUPLICATE_CUSTOM_TARGETS() const
 void cmGlobalGenerator::ComputeBuildFileGenerators()
 {
   for (unsigned int i = 0; i < this->LocalGenerators.size(); ++i) {
-    std::vector<cmExportBuildFileGenerator*> gens =
+    std::vector<std::unique_ptr<cmExportBuildFileGenerator>> const& gens =
       this->Makefiles[i]->GetExportBuildFileGenerators();
-    for (cmExportBuildFileGenerator* g : gens) {
+    for (std::unique_ptr<cmExportBuildFileGenerator> const& g : gens) {
       g->Compute(this->LocalGenerators[i].get());
     }
   }