Merge pull request #25 from ne-foss-org/nebuild-patches-deref

[CHORE] Patching TOML manifest parser to avoid null deref.

3 files changed, 64 insertions, 57 deletions

diff --git a/src/CommandLine/CLI.cpp b/src/CommandLine/CLI.cpp
index 5645728..b942178 100644
--- a/src/CommandLine/CLI.cpp
+++ b/src/CommandLine/CLI.cpp
@@ -9,8 +9,8 @@
 #include <NeBuildKit/JSONManifestBuilder.h>
 #include <NeBuildKit/TOMLManifestBuilder.h>
 #include <memory>
-#include <thread>
 #include <mutex>
+#include <thread>
 #include <vector>
 
 constexpr auto kNeBuildFileJson = "Jbuild.json";
@@ -22,7 +22,7 @@ int main(int argc, char** argv) {
   NeBuild::BuildConfig config;
 
   std::vector<std::thread> jobs;
-  
+
   for (size_t index{1}; index < argc; ++index) {
     std::string index_path = argv[index];
 
@@ -37,57 +37,58 @@ int main(int argc, char** argv) {
       return EXIT_SUCCESS;
     }
 
-    auto index_cpy = index;
+    auto       index_cpy = index;
     std::mutex mutex;
 
-    jobs.push_back(std::thread{[&mutex, &index, &index_cpy, &argc, &argv, &config](std::string index_path) -> void {
-      std::unique_lock<decltype(mutex)> lk{mutex};
-      std::unique_ptr<NeBuild::IManifestBuilder> builder;
+    jobs.push_back(std::thread{
+        [&mutex, &index, &index_cpy, &argc, &argv, &config](std::string index_path) -> void {
+          std::unique_lock<decltype(mutex)>          lk{mutex};
+          std::unique_ptr<NeBuild::IManifestBuilder> builder;
 
-      constexpr auto kJsonExtension = ".json";
+          constexpr auto kJsonExtension = ".json";
 
-      if (index_path.ends_with(kJsonExtension) || index_path == kNeBuildFileJson) {
-        builder = std::make_unique<NeBuild::JSONManifestBuilder>();
+          if (index_path.ends_with(kJsonExtension) || index_path == kNeBuildFileJson) {
+            builder = std::make_unique<NeBuild::JSONManifestBuilder>();
 
-	/// report failed build to config.
-        if (!builder) {
-          config.has_failed(true);
-          return;
-        }
-      } else {
-        constexpr auto kTomlExtension = ".toml";
-        builder = std::make_unique<NeBuild::TOMLManifestBuilder>();
+            /// report failed build to config.
+            if (!builder) {
+              config.has_failed(true);
+              return;
+            }
+          } else {
+            constexpr auto kTomlExtension = ".toml";
+            builder                       = std::make_unique<NeBuild::TOMLManifestBuilder>();
 
-        if (!index_path.ends_with(kTomlExtension) && index_path != kNeBuildFileToml) {
-          NeBuild::Logger::info() << "error: file '" << index_path << "' is not a manifest file!"
-                                  << std::endl;
-          config.has_failed(true);
-          return;
-        }
-      }
+            if (!index_path.ends_with(kTomlExtension) && index_path != kNeBuildFileToml) {
+              NeBuild::Logger::info()
+                  << "error: file '" << index_path << "' is not a manifest file!" << std::endl;
+              config.has_failed(true);
+              return;
+            }
+          }
 
-      std::string next_path;
+          std::string next_path;
 
-      if ((index_cpy + 1) < argc && argv[index_cpy + 1]) next_path = argv[index_cpy + 1];
+          if ((index_cpy + 1) < argc && argv[index_cpy + 1]) next_path = argv[index_cpy + 1];
 
-      if (next_path == "-build-system") {
-        NeBuild::Logger::info() << builder->BuildSystem() << std::endl;
-        std::exit(EXIT_SUCCESS);
-      }
+          if (next_path == "-build-system") {
+            NeBuild::Logger::info() << builder->BuildSystem() << std::endl;
+            std::exit(EXIT_SUCCESS);
+          }
 
-      NeBuild::Logger::info() << "building manifest: " << index_path << std::endl;
+          NeBuild::Logger::info() << "building manifest: " << index_path << std::endl;
 
-      config.path(index_path);
+          config.path(index_path);
 
-      if (builder && !builder->BuildTarget(config)) {
-        config.has_failed(true);
-      }
-    }, index_path});
+          if (builder && !builder->BuildTarget(config)) {
+            config.has_failed(true);
+          }
+        },
+        index_path});
   }
 
-  for (auto& job : jobs)
-    job.join();
-  
+  for (auto& job : jobs) job.join();
+
   // check for whether config is valid. if so return failure, or success.
   return !config ? EXIT_FAILURE : EXIT_SUCCESS;
 }
diff --git a/src/NeBuildKit/JSONManifestBuilder.cpp b/src/NeBuildKit/JSONManifestBuilder.cpp
index 0cc9fd3..b3db08f 100644
--- a/src/NeBuildKit/JSONManifestBuilder.cpp
+++ b/src/NeBuildKit/JSONManifestBuilder.cpp
@@ -28,8 +28,7 @@ bool JSONManifestBuilder::BuildTarget(BuildConfig& config) {
     path = config.path_;
 
     if (!FS::exists(path)) {
-      NeBuild::Logger::info() << "error: file '" << path << "' does not exist"
-                              << std::endl;
+      NeBuild::Logger::info() << "error: file '" << path << "' does not exist" << std::endl;
       return false;
     }
   }
@@ -38,8 +37,8 @@ bool JSONManifestBuilder::BuildTarget(BuildConfig& config) {
     std::ifstream json(path);
 
     if (!json.good()) {
-      NeBuild::Logger::info() << "error: file '" << path
-                              << "' is not a valid nlohmann::json" << std::endl;
+      NeBuild::Logger::info() << "error: file '" << path << "' is not a valid nlohmann::json"
+                              << std::endl;
       return false;
     }
 
@@ -52,7 +51,8 @@ bool JSONManifestBuilder::BuildTarget(BuildConfig& config) {
 
       if (auto res = description.get<std::string>(); !res.empty())
         NeBuild::Logger::info() << "description: " << res << std::endl;
-    } catch (...) {}
+    } catch (...) {
+    }
 
     std::string compiler = json_obj["compiler_path"].get<std::string>();
 
diff --git a/src/NeBuildKit/TOMLManifestBuilder.cpp b/src/NeBuildKit/TOMLManifestBuilder.cpp
index cce9384..d7cb585 100644
--- a/src/NeBuildKit/TOMLManifestBuilder.cpp
+++ b/src/NeBuildKit/TOMLManifestBuilder.cpp
@@ -28,30 +28,36 @@ bool TOMLManifestBuilder::BuildTarget(BuildConfig& config) {
     path = config.path_;
 
     if (!FS::exists(path)) {
-      NeBuild::Logger::info() << "error: file '" << path << "' does not exist"
-                              << std::endl;
+      NeBuild::Logger::info() << "error: file '" << path << "' does not exist" << std::endl;
       return false;
     }
   }
-    
+
   try {
     auto toml_file = toml::parse_file(path);
 
     try {
-      auto* description = toml_file["description"].as_string(); 
-      
+      auto* description = toml_file["description"].as_string();
+
+      if (!description) throw std::runtime_error({});
+
       NeBuild::Logger::info() << "package path: " << path << std::endl;
-      
-      if (description) NeBuild::Logger::info() << "description: " << description->get() << std::endl;
+
+      if (description)
+        NeBuild::Logger::info() << "description: " << description->get() << std::endl;
     } catch (...) {
-      // ...
     }
-    
-    std::string compiler = toml_file["compiler_path"].as_string()->get();
 
-    std::string command = compiler + " ";
+    auto* compiler_path_ptr = toml_file["compiler_path"].as_string();
+    if (!compiler_path_ptr) return false;
+
+    std::string compiler = compiler_path_ptr->get();
+    std::string command  = compiler + " ";
 
-    auto header_search_path = toml_file["compiler_headers_path"].as_array();
+    auto* header_search_path_ptr = toml_file["compiler_headers_path"].as_array();
+    if (!header_search_path_ptr) return false;
+
+    auto header_search_path = header_search_path_ptr;
 
     if (header_search_path) {
       for (auto& headers : *header_search_path) {
@@ -126,5 +132,5 @@ bool TOMLManifestBuilder::BuildTarget(BuildConfig& config) {
 const std::string_view TOMLManifestBuilder::BuildSystem() {
   return "NeBuild (toml++::toml)";
 }
-  
+
 }  // namespace NeBuild