From f3da9aa73e1639f6f768924757ba39f065ff50f1 Mon Sep 17 00:00:00 2001
From: Zach Wasserman <zach@fleetdm.com>
Date: Fri, 3 May 2024 05:03:40 -0700
Subject: [PATCH] Fix crash when carve size is stored as string (#8297)

---
 osquery/carver/carver.cpp          |  2 +-
 osquery/carver/carver_utils.cpp    | 32 ------------------------------
 osquery/carver/carver_utils.h      | 32 +++++++++++++++++++++++++++++-
 osquery/tables/forensic/carves.cpp | 13 +++++++++---
 4 files changed, 42 insertions(+), 37 deletions(-)

diff --git a/osquery/carver/carver.cpp b/osquery/carver/carver.cpp
index 494e3c496ed..c6f751999d4 100644
--- a/osquery/carver/carver.cpp
+++ b/osquery/carver/carver.cpp
@@ -204,7 +204,7 @@ Status Carver::carve() {
   }
 
   PlatformFile uploadFile(uploadPath, PF_OPEN_EXISTING | PF_READ);
-  updateCarveValue(carveGuid_, "size", std::to_string(uploadFile.size()));
+  updateCarveValue(carveGuid_, "size", uploadFile.size());
 
   std::string uploadHash =
       (uploadFile.size() > FLAGS_read_max)
diff --git a/osquery/carver/carver_utils.cpp b/osquery/carver/carver_utils.cpp
index 96a9de6448f..712e156952d 100644
--- a/osquery/carver/carver_utils.cpp
+++ b/osquery/carver/carver_utils.cpp
@@ -29,38 +29,6 @@ CLI_FLAG(bool,
 
 std::atomic<bool> kCarverPendingCarves{true};
 
-/// Helper function to update values related to a carve
-void updateCarveValue(const std::string& guid,
-                      const std::string& key,
-                      const std::string& value) {
-  std::string carve;
-  auto s = getDatabaseValue(kCarves, kCarverDBPrefix + guid, carve);
-  if (!s.ok()) {
-    VLOG(1) << "Failed to update status of carve in database " << guid;
-    return;
-  }
-
-  JSON tree;
-  s = tree.fromString(carve);
-  if (!s.ok()) {
-    VLOG(1) << "Failed to parse carve entries: " << s.what();
-    return;
-  }
-
-  tree.add(key, value);
-
-  std::string out;
-  s = tree.toString(out);
-  if (!s.ok()) {
-    VLOG(1) << "Failed to serialize carve entries: " << s.what();
-  }
-
-  s = setDatabaseValue(kCarves, kCarverDBPrefix + guid, out);
-  if (!s.ok()) {
-    VLOG(1) << "Failed to update status of carve in database " << guid;
-  }
-}
-
 std::string createCarveGuid() {
   return boost::uuids::to_string(boost::uuids::random_generator()());
 }
diff --git a/osquery/carver/carver_utils.h b/osquery/carver/carver_utils.h
index cef3e1767ae..87ea6d7a099 100644
--- a/osquery/carver/carver_utils.h
+++ b/osquery/carver/carver_utils.h
@@ -9,6 +9,8 @@
 
 #pragma once
 
+#include <osquery/database/database.h>
+#include <osquery/logger/logger.h>
 #include <osquery/utils/status/status.h>
 
 #include <atomic>
@@ -46,9 +48,37 @@ const std::string kCarverStatusScheduled = "SCHEDULED";
 extern std::atomic<bool> kCarverPendingCarves;
 
 /// Update an attribute for a given carve GUID.
+template <typename T>
 void updateCarveValue(const std::string& guid,
                       const std::string& key,
-                      const std::string& value);
+                      const T& value) {
+  std::string carve;
+  auto s = getDatabaseValue(kCarves, kCarverDBPrefix + guid, carve);
+  if (!s.ok()) {
+    VLOG(1) << "Failed to update status of carve in database " << guid;
+    return;
+  }
+
+  JSON tree;
+  s = tree.fromString(carve);
+  if (!s.ok()) {
+    VLOG(1) << "Failed to parse carve entries: " << s.what();
+    return;
+  }
+
+  tree.add(key, value);
+
+  std::string out;
+  s = tree.toString(out);
+  if (!s.ok()) {
+    VLOG(1) << "Failed to serialize carve entries: " << s.what();
+  }
+
+  s = setDatabaseValue(kCarves, kCarverDBPrefix + guid, out);
+  if (!s.ok()) {
+    VLOG(1) << "Failed to update status of carve in database " << guid;
+  }
+}
 
 /// Returns a UUID.
 std::string createCarveGuid();
diff --git a/osquery/tables/forensic/carves.cpp b/osquery/tables/forensic/carves.cpp
index 1fcdb25af17..9c0b366a47e 100644
--- a/osquery/tables/forensic/carves.cpp
+++ b/osquery/tables/forensic/carves.cpp
@@ -54,7 +54,14 @@ void enumerateCarves(QueryData& results, const std::string& new_guid) {
     }
 
     if (tree.doc().HasMember("size")) {
-      r["size"] = INTEGER(tree.doc()["size"].GetInt());
+      // In some instances, it was observed that the size is stored as a JSON
+      // string rather than number, resulting in an exception if GetInt was
+      // called. Check for the type for backwards compatibility.
+      if (tree.doc()["size"].IsInt()) {
+        r["size"] = INTEGER(tree.doc()["size"].GetInt());
+      } else {
+        r["size"] = INTEGER(tree.doc()["size"].GetString());
+      }
     }
 
     stringToRow("sha256", r, tree);
@@ -108,5 +115,5 @@ QueryData genCarves(QueryContext& context) {
 
   return results;
 }
-}
-}
+} // namespace tables
+} // namespace osquery