cmPackageInfoReader: Don't crash if input is malformed

Check all instances of converting a JSON value to a string to ensure
that we check first if the value is convertible, in order to avoid an
exception being thrown, which crashes CMake. Modify some instances to
report when we encounter such invalid values. (Many instances, however,
just silently ignore invalid values.)

Fixes: #27350

Source/cmFindPackageCommand.cxx

@@ -2055,8 +2055,11 @@ cmFindPackageCommand::AppendixMap cmFindPackageCommand::FindAppendices(
         continue;
       }
 
+      cmMakefile::CallRAII cs{ this->Makefile, extra, this->Status };
+
       std::unique_ptr<cmPackageInfoReader> reader =
-        cmPackageInfoReader::Read(extra, &baseReader);
+        cmPackageInfoReader::Read(this->Makefile, extra, &baseReader);
+
       if (reader && reader->GetName() == this->Name) {
         std::vector<std::string> components = reader->GetComponentNames();
         Appendix appendix{ std::move(reader), std::move(components) };
@@ -2249,8 +2252,10 @@ bool cmFindPackageCommand::ImportPackageTargets(cmPackageState& packageState,
 
     // Try to read supplemental data from each file found.
     for (std::string const& extra : glob.GetFiles()) {
+      cmMakefile::CallRAII cs{ this->Makefile, extra, this->Status };
+
       std::unique_ptr<cmPackageInfoReader> configReader =
-        cmPackageInfoReader::Read(extra, &reader);
+        cmPackageInfoReader::Read(this->Makefile, extra, &reader);
       if (configReader && configReader->GetName() == this->Name) {
         if (!configReader->ImportTargetConfigurations(this->Makefile,
                                                       this->Status)) {
@@ -2953,8 +2958,11 @@ bool cmFindPackageCommand::CheckVersion(std::string const& config_file)
   std::string ext = cmSystemTools::LowerCase(config_file.substr(pos));
 
   if (ext == ".cps"_s) {
+    cmMakefile::CallRAII cs{ this->Makefile, config_file, this->Status };
+
     std::unique_ptr<cmPackageInfoReader> reader =
-      cmPackageInfoReader::Read(config_file);
+      cmPackageInfoReader::Read(this->Makefile, config_file);
+
     if (reader && reader->GetName() == this->Name) {
       // Read version information.
       cm::optional<std::string> cpsVersion = reader->GetVersion();

Source/cmPackageInfoReader.cxx

@@ -109,9 +109,17 @@ Json::Value ReadJson(std::string const& fileName)
   return data;
 }
 
+std::string ToString(Json::Value const& value)
+{
+  if (value.isString()) {
+    return value.asString();
+  }
+  return {};
+}
+
 bool CheckSchemaVersion(Json::Value const& data)
 {
-  std::string const& version = data["cps_version"].asString();
+  std::string const& version = ToString(data["cps_version"]);
 
   // Check that a valid version is specified.
   if (version.empty()) {
@@ -137,7 +145,7 @@ std::string DeterminePrefix(std::string const& filepath,
                             Json::Value const& data)
 {
   // First check if an absolute prefix was supplied.
-  std::string prefix = data["prefix"].asString();
+  std::string prefix = ToString(data["prefix"]);
   if (!prefix.empty()) {
     // Ensure that the specified prefix is valid.
     if (cmsys::SystemTools::FileIsFullPath(prefix) &&
@@ -151,7 +159,7 @@ std::string DeterminePrefix(std::string const& filepath,
 
   // Get and validate prefix-relative path.
   std::string const& absPath = cmSystemTools::GetFilenamePath(filepath);
-  std::string relPath = data["cps_path"].asString();
+  std::string relPath = ToString(data["cps_path"]);
   cmSystemTools::ConvertToUnixSlashes(relPath);
   if (relPath.empty() || !cmHasLiteralPrefix(relPath, "@prefix@")) {
     // The relative prefix is not valid.
@@ -436,7 +444,8 @@ cm::optional<cmPackageInfoReader::Pep440Version> ParseSimpleVersion(
 } // namespace
 
 std::unique_ptr<cmPackageInfoReader> cmPackageInfoReader::Read(
-  std::string const& path, cmPackageInfoReader const* parent)
+  cmMakefile* makefile, std::string const& path,
+  cmPackageInfoReader const* parent)
 {
   // Read file and perform some basic validation:
   //   - the input is valid JSON
@@ -485,7 +494,13 @@ std::unique_ptr<cmPackageInfoReader> cmPackageInfoReader::Read(
   // Check for a default license.
   Json::Value const& defaultLicense = reader->Data["default_license"];
   if (!defaultLicense.isNull()) {
-    reader->DefaultLicense = defaultLicense.asString();
+    if (defaultLicense.isString()) {
+      reader->DefaultLicense = defaultLicense.asString();
+    } else {
+      makefile->IssueMessage(
+        MessageType::WARNING,
+        "Package attribute \"default_license\" is not a string.");
+    }
   } else if (parent) {
     reader->DefaultLicense = parent->DefaultLicense;
   } else {
@@ -495,7 +510,13 @@ std::unique_ptr<cmPackageInfoReader> cmPackageInfoReader::Read(
     // consistent with not allowing LICENSE and APPENDIX to be used together.
     Json::Value const& packageLicense = reader->Data["license"];
     if (!packageLicense.isNull()) {
-      reader->DefaultLicense = packageLicense.asString();
+      if (packageLicense.isString()) {
+        reader->DefaultLicense = packageLicense.asString();
+      } else {
+        makefile->IssueMessage(
+          MessageType::WARNING,
+          "Package attribute \"license\" is not a string.");
+      }
     }
   }
 
@@ -504,7 +525,7 @@ std::unique_ptr<cmPackageInfoReader> cmPackageInfoReader::Read(
 
 std::string cmPackageInfoReader::GetName() const
 {
-  return this->Data["name"].asString();
+  return ToString(this->Data["name"]);
 }
 
 cm::optional<std::string> cmPackageInfoReader::GetVersion() const
@@ -536,7 +557,7 @@ cmPackageInfoReader::ParseVersion(
 
   // Check if we know how to parse the version.
   Json::Value const& schema = this->Data["version_schema"];
-  if (schema.isNull() || cmStrCaseEq(schema.asString(), "simple"_s)) {
+  if (schema.isNull() || cmStrCaseEq(ToString(schema), "simple"_s)) {
     return ParseSimpleVersion(*version);
   }
 
@@ -551,7 +572,7 @@ std::vector<cmPackageRequirement> cmPackageInfoReader::GetRequirements() const
 
   for (auto ri = requirementObjects.begin(), re = requirementObjects.end();
        ri != re; ++ri) {
-    cmPackageRequirement r{ ri.name(), (*ri)["version"].asString(),
+    cmPackageRequirement r{ ri.name(), ToString((*ri)["version"]),
                             ReadList(*ri, "components"),
                             ReadList(*ri, "hints") };
     requirements.emplace_back(std::move(r));
@@ -627,11 +648,14 @@ void cmPackageInfoReader::AddTargetConfiguration(
   target->SetProperty("IMPORTED_CONFIGURATIONS", cmJoin(newConfigs, ";"_s));
 }
 
-void cmPackageInfoReader::SetImportProperty(cmTarget* target,
+void cmPackageInfoReader::SetImportProperty(cmMakefile* makefile,
+                                            cmTarget* target,
                                             cm::string_view property,
                                             cm::string_view configuration,
-                                            Json::Value const& value) const
+                                            Json::Value const& object,
+                                            std::string const& attribute) const
 {
+  Json::Value const& value = object[attribute];
   if (!value.isNull()) {
     std::string fullprop;
     if (configuration.empty()) {
@@ -641,16 +665,34 @@ void cmPackageInfoReader::SetImportProperty(cmTarget* target,
                           cmSystemTools::UpperCase(configuration));
     }
 
-    target->SetProperty(fullprop, this->ResolvePath(value.asString()));
+    if (value.isString()) {
+      target->SetProperty(fullprop, this->ResolvePath(value.asString()));
+    } else {
+      makefile->IssueMessage(MessageType::WARNING,
+                             cmStrCat("Failed to set property \""_s, property,
+                                      "\" on target \""_s, target->GetName(),
+                                      "\": attribute \"", attribute,
+                                      "\" is not a string."_s));
+    }
   }
 }
 
 void cmPackageInfoReader::SetMetaProperty(
-  cmTarget* target, std::string const& property, Json::Value const& value,
+  cmMakefile* makefile, cmTarget* target, std::string const& property,
+  Json::Value const& object, std::string const& attribute,
   std::string const& defaultValue) const
 {
+  Json::Value const& value = object[attribute];
   if (!value.isNull()) {
-    target->SetProperty(property, value.asString());
+    if (value.isString()) {
+      target->SetProperty(property, value.asString());
+    } else {
+      makefile->IssueMessage(MessageType::WARNING,
+                             cmStrCat("Failed to set property \""_s, property,
+                                      "\" on target \""_s, target->GetName(),
+                                      "\": attribute \"", attribute,
+                                      "\" is not a string."_s));
+    }
   } else if (!defaultValue.empty()) {
     target->SetProperty(property, defaultValue);
   }
@@ -702,14 +744,14 @@ void cmPackageInfoReader::SetTargetProperties(
                            });
 
   // Add link name/location(s).
-  this->SetImportProperty(target, "LOCATION"_s, configuration,
-                          data["location"]);
+  this->SetImportProperty(makefile, target, "LOCATION"_s, // br
+                          configuration, data, "location");
 
-  this->SetImportProperty(target, "IMPLIB"_s, configuration,
-                          data["link_location"]);
+  this->SetImportProperty(makefile, target, "IMPLIB"_s, // br
+                          configuration, data, "link_location");
 
-  this->SetImportProperty(target, "SONAME"_s, configuration,
-                          data["link_name"]);
+  this->SetImportProperty(makefile, target, "SONAME"_s, // br
+                          configuration, data, "link_name");
 
   // Add link languages.
   for (std::string const& originalLang : ReadList(data, "link_languages")) {
@@ -734,7 +776,7 @@ void cmPackageInfoReader::SetTargetProperties(
 
   // Add other information.
   if (configuration.empty()) {
-    this->SetMetaProperty(target, "SPDX_LICENSE", data["license"],
+    this->SetMetaProperty(makefile, target, "SPDX_LICENSE", data, "license",
                           this->DefaultLicense);
   }
 }
@@ -768,7 +810,7 @@ bool cmPackageInfoReader::ImportTargets(cmMakefile* makefile,
   for (auto ci = components.begin(), ce = components.end(); ci != ce; ++ci) {
     cm::string_view const name = IterKey(ci);
     std::string const& type =
-      cmSystemTools::LowerCase((*ci)["type"].asString());
+      cmSystemTools::LowerCase(ToString((*ci)["type"]));
 
     // Get and validate full target name.
     std::string const& fullName = cmStrCat(package, "::"_s, name);
@@ -833,7 +875,7 @@ bool cmPackageInfoReader::ImportTargets(cmMakefile* makefile,
 bool cmPackageInfoReader::ImportTargetConfigurations(
   cmMakefile* makefile, cmExecutionStatus& status) const
 {
-  std::string const& configuration = this->Data["configuration"].asString();
+  std::string const& configuration = ToString(this->Data["configuration"]);
 
   if (configuration.empty()) {
     makefile->IssueMessage(MessageType::WARNING,

Source/cmPackageInfoReader.h

@@ -39,7 +39,8 @@ class cmPackageInfoReader
 {
 public:
   static std::unique_ptr<cmPackageInfoReader> Read(
-    std::string const& path, cmPackageInfoReader const* parent = nullptr);
+    cmMakefile* makefile, std::string const& path,
+    cmPackageInfoReader const* parent = nullptr);
 
   std::string GetName() const;
   cm::optional<std::string> GetVersion() const;
@@ -97,11 +98,14 @@ private:
   void SetTargetProperties(cmMakefile* makefile, cmTarget* target,
                            Json::Value const& data, std::string const& package,
                            cm::string_view configuration) const;
-  void SetImportProperty(cmTarget* target, cm::string_view property,
+  void SetImportProperty(cmMakefile* makefile, cmTarget* target,
+                         cm::string_view property,
                          cm::string_view configuration,
-                         Json::Value const& value) const;
-  void SetMetaProperty(cmTarget* target, std::string const& property,
-                       Json::Value const& value,
+                         Json::Value const& object,
+                         std::string const& attribute) const;
+  void SetMetaProperty(cmMakefile* makefile, cmTarget* target,
+                       std::string const& property, Json::Value const& object,
+                       std::string const& attribute,
                        std::string const& defaultValue = {}) const;
 
   std::string ResolvePath(std::string path) const;

Tests/RunCMake/find_package-CPS/InvalidCps3-result.txt

@@ -0,0 +1 @@
+0

Tests/RunCMake/find_package-CPS/InvalidCps3-stderr.txt

@@ -0,0 +1,13 @@
+CMake Warning in cps/[Cc]orrupt\.cps:
+  Package attribute "license" is not a string\.
+Call Stack \(most recent call first\):
+  InvalidCps3\.cmake:10 \(find_package\)
+  CMakeLists\.txt:3 \(include\)
+
+
+CMake Warning in cps/[Cc]orrupt\.cps:
+  Failed to set property "IMPLIB" on target "Corrupt::corrupt": attribute
+  "link_location" is not a string\.
+Call Stack \(most recent call first\):
+  InvalidCps3\.cmake:10 \(find_package\)
+  CMakeLists\.txt:3 \(include\)

Tests/RunCMake/find_package-CPS/InvalidCps3.cmake

@@ -0,0 +1,10 @@
+cmake_minimum_required(VERSION 4.0)
+
+include(Setup.cmake)
+
+set(CMAKE_FIND_PACKAGE_SORT_ORDER NAME)
+set(CMAKE_FIND_PACKAGE_SORT_DIRECTION DEC)
+
+###############################################################################
+# Test reporting when trying to read a .cps that is not schema conforming.
+find_package(Corrupt REQUIRED)

Tests/RunCMake/find_package-CPS/RunCMakeTest.cmake

@@ -32,6 +32,7 @@ endfunction()
 # General failure tests
 run_cmake(InvalidCps1)
 run_cmake(InvalidCps2)
+run_cmake(InvalidCps3)
 run_cmake(WrongName)
 run_cmake(BadPrefix)
 

Tests/RunCMake/find_package-CPS/cps/corrupt.cps

@@ -0,0 +1,12 @@
+{
+  "cps_version": "0.13.0",
+  "name": "Corrupt",
+  "license": [],
+  "cps_path": "@prefix@/cps",
+  "components": {
+    "corrupt": {
+      "type": "archive",
+      "link_location": []
+    }
+  }
+}