From 32c5ce74ff4d353c2df0277c0a31ab32130ae432 Mon Sep 17 00:00:00 2001
From: Oliver Hamlet <oliver@ortham.io>
Date: Thu, 29 Dec 2022 23:41:49 +0000
Subject: [PATCH] Fix trying to dereference null pointer after bad cast

Fortunately this never actually happens.
---
 src/api/plugin.cpp                    | 46 +++++++++++++--------------
 src/tests/api/internals/plugin_test.h | 21 +++++++++++-
 2 files changed, 43 insertions(+), 24 deletions(-)

diff --git a/src/api/plugin.cpp b/src/api/plugin.cpp
index 67771f6d..4d8e9135 100644
--- a/src/api/plugin.cpp
+++ b/src/api/plugin.cpp
@@ -197,34 +197,34 @@ size_t Plugin::GetOverlapSize(
     return 0;
   }
 
-  try {
-    std::vector<::Plugin*> esPlugins;
-    for (const auto& plugin : plugins) {
-      const auto otherPlugin = dynamic_cast<const Plugin* const>(plugin);
+  std::vector<::Plugin*> esPlugins;
+  for (const auto& plugin : plugins) {
+    const auto otherPlugin = dynamic_cast<const Plugin* const>(plugin);
 
-      esPlugins.push_back(otherPlugin->esPlugin.get());
-    }
-
-    size_t overlapSize = 0;
-    const auto ret = esp_plugin_records_overlap_size(
-        esPlugin.get(), esPlugins.data(), esPlugins.size(), &overlapSize);
-    if (ret != ESP_OK) {
-      throw FileAccessError("Error getting overlap size for \"" + name_ +
-                            "\". esplugin error code: " + std::to_string(ret));
-    }
-
-    return overlapSize;
-  } catch (std::bad_cast&) {
-    auto logger = getLogger();
-    if (logger) {
-      logger->error(
+    if (otherPlugin == nullptr) {
+      const auto logger = getLogger();
+      if (logger) {
+        logger->error(
+            "Tried to check how many FormIDs overlapped with a non-Plugin "
+            "implementation of PluginSortingInterface.");
+      }
+      throw std::invalid_argument(
           "Tried to check how many FormIDs overlapped with a non-Plugin "
           "implementation of PluginSortingInterface.");
     }
-    throw std::invalid_argument(
-        "Tried to check how many FormIDs overlapped with a non-Plugin "
-        "implementation of PluginSortingInterface.");
+
+    esPlugins.push_back(otherPlugin->esPlugin.get());
   }
+
+  size_t overlapSize = 0;
+  const auto ret = esp_plugin_records_overlap_size(
+      esPlugin.get(), esPlugins.data(), esPlugins.size(), &overlapSize);
+  if (ret != ESP_OK) {
+    throw FileAccessError("Error getting overlap size for \"" + name_ +
+                          "\". esplugin error code: " + std::to_string(ret));
+  }
+
+  return overlapSize;
 }
 
 size_t Plugin::NumOverrideFormIDs() const { return numOverrideRecords_; }
diff --git a/src/tests/api/internals/plugin_test.h b/src/tests/api/internals/plugin_test.h
index 20f80882..43a97a6e 100644
--- a/src/tests/api/internals/plugin_test.h
+++ b/src/tests/api/internals/plugin_test.h
@@ -136,7 +136,7 @@ private:
   }
 };
 
-class OtherPluginType final : public PluginInterface {
+class OtherPluginType final : public PluginSortingInterface {
 public:
   std::string GetName() const override { return ""; }
   std::optional<float> GetHeaderVersion() const override { return 0.0f; }
@@ -155,6 +155,14 @@ public:
   bool IsEmpty() const override { return false; }
   bool LoadsArchive() const override { return false; }
   bool DoFormIDsOverlap(const PluginInterface&) const override { return true; }
+
+  size_t NumOverrideFormIDs() const override { return 0; };
+  uint32_t GetRecordAndGroupCount() const override { return 0; };
+
+  size_t GetOverlapSize(
+      const std::vector<const PluginInterface*>&) const override {
+    return 0;
+  };
 };
 
 // Pass an empty first argument, as it's a prefix for the test instantation,
@@ -485,6 +493,17 @@ TEST_P(PluginTest,
   EXPECT_TRUE(plugin2.DoFormIDsOverlap(plugin1));
 }
 
+TEST_P(PluginTest,
+       getOverlapSizeShouldThrowIfGivenAVectorContainingANonPluginObject) {
+  Plugin plugin1(
+      game_.Type(), game_.GetCache(), game_.DataPath() / blankEsm, false);
+  OtherPluginType plugin2;
+
+  EXPECT_THROW(plugin1.GetOverlapSize({&plugin2, &plugin2}),
+               std::invalid_argument);
+  EXPECT_EQ(0, plugin2.GetOverlapSize({&plugin1}));
+}
+
 TEST_P(PluginTest, getOverlapSizeShouldCountEachRecordOnce) {
   Plugin plugin1(
       game_.Type(), game_.GetCache(), game_.DataPath() / blankEsm, false);