From 8b3465231ed33237898d985064f2c2b13056f67e Mon Sep 17 00:00:00 2001
From: dmiller <dmiller@e0a8ed71-7df4-0310-8962-fdc924857419>
Date: Tue, 13 Sep 2022 20:05:34 +0000
Subject: [PATCH] Fix crash: manage lifetime of now-dynamic test results

---
 FingerPrintResults.cc |  1 +
 osscan.cc             | 26 +++++++++++++++++++++++---
 osscan.h              | 15 +++++----------
 osscan2.cc            | 11 +++++++----
 4 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/FingerPrintResults.cc b/FingerPrintResults.cc
index 6f7b2a37a..f8bc4706f 100644
--- a/FingerPrintResults.cc
+++ b/FingerPrintResults.cc
@@ -94,6 +94,7 @@ FingerPrintResultsIPv4::~FingerPrintResultsIPv4() {
 
   /* Free OS fingerprints of OS scanning was done */
   for(i=0; i < numFPs; i++) {
+    FPs[i]->erase();
     delete(FPs[i]);
     FPs[i] = NULL;
   }
diff --git a/osscan.cc b/osscan.cc
index 013c2a62d..800f7d2bd 100644
--- a/osscan.cc
+++ b/osscan.cc
@@ -84,13 +84,26 @@ FingerPrintDB::FingerPrintDB() : MatchPoints(NULL) {
 FingerPrintDB::~FingerPrintDB() {
   std::vector<FingerPrint *>::iterator current;
 
-  if (MatchPoints != NULL)
+  if (MatchPoints != NULL) {
+    MatchPoints->erase();
     delete MatchPoints;
-  for (current = prints.begin(); current != prints.end(); current++)
+  }
+  for (current = prints.begin(); current != prints.end(); current++) {
+    (*current)->erase();
     delete *current;
+  }
 }
 
-FingerPrint::FingerPrint() {
+FingerTest::FingerTest(bool allocResults) : name(NULL), results(NULL) {
+  if (allocResults)
+    this->results = new std::vector<struct AVal>;
+}
+
+void FingerTest::erase() {
+  if (this->results) {
+    delete this->results;
+    this->results = NULL;
+  }
 }
 
 void FingerPrint::sort() {
@@ -101,6 +114,13 @@ void FingerPrint::sort() {
   std::stable_sort(tests.begin(), tests.end());
 }
 
+void FingerPrint::erase() {
+  for (std::vector<FingerTest>::iterator t = this->tests.begin();
+      t != this->tests.end(); t++) {
+    t->erase();
+  }
+}
+
 /* Compare an observed value (e.g. "45") against an OS DB expression (e.g.
    "3B-47" or "8|A" or ">10"). Return true iff there's a match. The syntax uses
      < (less than)
diff --git a/osscan.h b/osscan.h
index f9fd62407..30f72c24c 100644
--- a/osscan.h
+++ b/osscan.h
@@ -93,6 +93,7 @@ enum dist_calc_method {
 struct AVal {
   const char *attribute;
   const char *value;
+  AVal() : attribute(NULL), value(NULL) {}
 
   bool operator<(const AVal& other) const {
     return strcmp(attribute, other.attribute) < 0;
@@ -126,28 +127,22 @@ struct FingerMatch {
 struct FingerTest {
   const char *name;
   std::vector<struct AVal> *results;
-  FingerTest() : name(NULL), results(NULL) {}
+  FingerTest(bool allocResults=false);
   ~FingerTest() {
     // name is allocated from string_pool
-    // results freed via ~FingerPrint()
+    // results must be freed manually
     }
   bool operator<(const FingerTest& other) const {
     return strcmp(name, other.name) < 0;
   }
+  void erase();
 };
 
 struct FingerPrint {
   FingerMatch match;
   std::vector<FingerTest> tests;
-  FingerPrint();
-  ~FingerPrint() {
-    for (std::vector<FingerTest>::iterator t = this->tests.begin();
-        t != this->tests.end(); t++) {
-      if (t->results)
-        delete t->results;
-    }
-  }
   void sort();
+  void erase();
 };
 /* This structure contains the important data from the fingerprint
    database (nmap-os-db) */
diff --git a/osscan2.cc b/osscan2.cc
index ad091c8d0..6309e8d31 100644
--- a/osscan2.cc
+++ b/osscan2.cc
@@ -1029,8 +1029,10 @@ HostOsScanStats::~HostOsScanStats() {
   int i;
 
   for (i = 0; i < NUM_FPTESTS; i++) {
-    if (FPtests[i] != NULL)
+    if (FPtests[i] != NULL) {
       delete FPtests[i];
+      FPtests[i] = NULL;
+    }
   }
   for (i = 0; i < 6; i++) {
     if (TOps_AVs[i])
@@ -1148,9 +1150,10 @@ void HostOsScanStats::initScanStats() {
 
   FP = NULL;
   for (i = 0; i < NUM_FPTESTS; i++) {
-    if (FPtests[i] != NULL)
+    if (FPtests[i] != NULL) {
       delete FPtests[i];
-    FPtests[i] = NULL;
+      FPtests[i] = NULL;
+    }
   }
   for (i = 0; i < 6; i++) {
     if (TOps_AVs[i])
@@ -2048,7 +2051,7 @@ void HostOsScan::makeFP(HostOsScanStats *hss) {
       /* We create a Resp (response) attribute with value of N (no) because
          it is important here to note whether responses were or were not
          received */
-      hss->FPtests[i] = new FingerTest;
+      hss->FPtests[i] = new FingerTest(true);
       AV.attribute = "R";
       AV.value = "N";
       hss->FPtests[i]->results->push_back(AV);