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

8 files changed, 41 insertions, 28 deletions

diff --git a/Source/cmExportCommand.cxx b/Source/cmExportCommand.cxx
index b7cc193..e49c174 100644
--- a/Source/cmExportCommand.cxx
+++ b/Source/cmExportCommand.cxx
@@ -198,7 +198,6 @@ bool cmExportCommand(std::vector<std::string> const& args,
   } else {
     ebfg->SetTargets(targets);
   }
-  mf.AddExportBuildFileGenerator(ebfg.get());
   ebfg->SetExportOld(arguments.ExportOld);
 
   // Compute the set of configurations exported.
@@ -211,10 +210,11 @@ bool cmExportCommand(std::vector<std::string> const& args,
     ebfg->AddConfiguration(ct);
   }
   if (exportSet != nullptr) {
-    gg->AddBuildExportExportSet(std::move(ebfg));
+    gg->AddBuildExportExportSet(ebfg.get());
   } else {
-    gg->AddBuildExportSet(std::move(ebfg));
+    gg->AddBuildExportSet(ebfg.get());
   }
+  mf.AddExportBuildFileGenerator(std::move(ebfg));
 
   return true;
 }
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());
     }
   }
diff --git a/Source/cmGlobalGenerator.h b/Source/cmGlobalGenerator.h
index ba997b2..7dc4822 100644
--- a/Source/cmGlobalGenerator.h
+++ b/Source/cmGlobalGenerator.h
@@ -463,13 +463,12 @@ public:
 
   void ProcessEvaluationFiles();
 
-  std::map<std::string, std::unique_ptr<cmExportBuildFileGenerator>>&
-  GetBuildExportSets()
+  std::map<std::string, cmExportBuildFileGenerator*>& GetBuildExportSets()
   {
     return this->BuildExportSets;
   }
-  void AddBuildExportSet(std::unique_ptr<cmExportBuildFileGenerator>);
-  void AddBuildExportExportSet(std::unique_ptr<cmExportBuildFileGenerator>);
+  void AddBuildExportSet(cmExportBuildFileGenerator* gen);
+  void AddBuildExportExportSet(cmExportBuildFileGenerator* gen);
   bool IsExportedTargetsFile(const std::string& filename) const;
   bool GenerateImportFile(const std::string& file);
   cmExportBuildFileGenerator* GetExportedTargetsFile(
@@ -580,8 +579,7 @@ protected:
   std::set<std::string> InstallComponents;
   // Sets of named target exports
   cmExportSetMap ExportSets;
-  std::map<std::string, std::unique_ptr<cmExportBuildFileGenerator>>
-    BuildExportSets;
+  std::map<std::string, cmExportBuildFileGenerator*> BuildExportSets;
   std::map<std::string, cmExportBuildFileGenerator*> BuildExportExportSets;
 
   std::map<std::string, std::string> AliasTargets;
diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index b2e59bd..18689fa 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -32,6 +32,7 @@
 #include "cmCustomCommandLines.h"
 #include "cmExecutionStatus.h"
 #include "cmExpandedCommandArgument.h" // IWYU pragma: keep
+#include "cmExportBuildFileGenerator.h"
 #include "cmFileLockPool.h"
 #include "cmFunctionBlocker.h"
 #include "cmGeneratedFileStream.h"
@@ -780,7 +781,7 @@ cmMakefile::GetEvaluationFiles() const
   return this->EvaluationFiles;
 }
 
-std::vector<cmExportBuildFileGenerator*>
+std::vector<std::unique_ptr<cmExportBuildFileGenerator>> const&
 cmMakefile::GetExportBuildFileGenerators() const
 {
   return this->ExportBuildFileGenerators;
@@ -789,16 +790,21 @@ cmMakefile::GetExportBuildFileGenerators() const
 void cmMakefile::RemoveExportBuildFileGeneratorCMP0024(
   cmExportBuildFileGenerator* gen)
 {
-  auto it = std::find(this->ExportBuildFileGenerators.begin(),
-                      this->ExportBuildFileGenerators.end(), gen);
+  auto it =
+    std::find_if(this->ExportBuildFileGenerators.begin(),
+                 this->ExportBuildFileGenerators.end(),
+                 [gen](std::unique_ptr<cmExportBuildFileGenerator> const& p) {
+                   return p.get() == gen;
+                 });
   if (it != this->ExportBuildFileGenerators.end()) {
     this->ExportBuildFileGenerators.erase(it);
   }
 }
 
-void cmMakefile::AddExportBuildFileGenerator(cmExportBuildFileGenerator* gen)
+void cmMakefile::AddExportBuildFileGenerator(
+  std::unique_ptr<cmExportBuildFileGenerator> gen)
 {
-  this->ExportBuildFileGenerators.push_back(gen);
+  this->ExportBuildFileGenerators.emplace_back(std::move(gen));
 }
 
 namespace {
diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h
index f1a68c2..d918abe 100644
--- a/Source/cmMakefile.h
+++ b/Source/cmMakefile.h
@@ -945,10 +945,11 @@ public:
   const std::vector<std::unique_ptr<cmGeneratorExpressionEvaluationFile>>&
   GetEvaluationFiles() const;
 
-  std::vector<cmExportBuildFileGenerator*> GetExportBuildFileGenerators()
-    const;
+  std::vector<std::unique_ptr<cmExportBuildFileGenerator>> const&
+  GetExportBuildFileGenerators() const;
   void RemoveExportBuildFileGeneratorCMP0024(cmExportBuildFileGenerator* gen);
-  void AddExportBuildFileGenerator(cmExportBuildFileGenerator* gen);
+  void AddExportBuildFileGenerator(
+    std::unique_ptr<cmExportBuildFileGenerator> gen);
 
   // Maintain a stack of package roots to allow nested PACKAGE_ROOT_PATH
   // searches
@@ -1053,7 +1054,8 @@ private:
   mutable cmsys::RegularExpression cmNamedCurly;
 
   std::vector<cmMakefile*> UnConfiguredDirectories;
-  std::vector<cmExportBuildFileGenerator*> ExportBuildFileGenerators;
+  std::vector<std::unique_ptr<cmExportBuildFileGenerator>>
+    ExportBuildFileGenerators;
 
   std::vector<std::unique_ptr<cmGeneratorExpressionEvaluationFile>>
     EvaluationFiles;
diff --git a/Tests/RunCMake/export/Repeat.cmake b/Tests/RunCMake/export/Repeat.cmake
new file mode 100644
index 0000000..f3262e7
--- /dev/null
+++ b/Tests/RunCMake/export/Repeat.cmake
@@ -0,0 +1,5 @@
+add_library(foo INTERFACE)
+export(TARGETS foo FILE foo.cmake)
+export(TARGETS foo FILE foo.cmake)
+add_subdirectory(Repeat)
+include(CMakePackageConfigHelpers)
diff --git a/Tests/RunCMake/export/Repeat/CMakeLists.txt b/Tests/RunCMake/export/Repeat/CMakeLists.txt
new file mode 100644
index 0000000..b37f6ca
--- /dev/null
+++ b/Tests/RunCMake/export/Repeat/CMakeLists.txt
@@ -0,0 +1,2 @@
+add_library(bar INTERFACE)
+export(TARGETS bar FILE ${CMAKE_BINARY_DIR}/foo.cmake)
diff --git a/Tests/RunCMake/export/RunCMakeTest.cmake b/Tests/RunCMake/export/RunCMakeTest.cmake
index 4d2f217..df35d95 100644
--- a/Tests/RunCMake/export/RunCMakeTest.cmake
+++ b/Tests/RunCMake/export/RunCMakeTest.cmake
@@ -2,6 +2,7 @@ include(RunCMake)
 
 run_cmake(CustomTarget)
 run_cmake(Empty)
+run_cmake(Repeat)
 run_cmake(TargetNotFound)
 run_cmake(AppendExport)
 run_cmake(OldIface)