fix(adhoc-sweep-fixes): CU-86aknprfh 11 review findings across 11 files

osquery/events/darwin/diskarbitration.cpp

@@ -269,3 +269,4 @@ std::string DiskArbitrationEventPublisher::getProperty(
   return "";
 }
 } // namespace osquery
+

osquery/events/eventsubscriberplugin.cpp

@@ -123,7 +123,6 @@ Status EventSubscriberPlugin::addBatch(std::vector<Row>& row_list,
 
   for (auto& row : row_list) {
     auto event_identifier = getEventID();
-    event_id_list.push_back(event_identifier);
 
     auto string_event_identifier = toIndex(event_identifier);
 
@@ -138,6 +137,11 @@ Status EventSubscriberPlugin::addBatch(std::vector<Row>& row_list,
       continue;
     }
 
+    // Only commit the event id to the index once its row data has been
+    // successfully serialized, so that the index never references a row
+    // that was never stored.
+    event_id_list.push_back(event_identifier);
+
     // Then remove the newline.
     if (serialized_row.size() > 0 && serialized_row.back() == '\n') {
       serialized_row.pop_back();

osquery/events/windows/windowseventlogpublisher.cpp

@@ -208,6 +208,10 @@ double WindowsEventLogPublisher::cosineSimilarity(
   std::vector<double> buffer_freqs(kCharFreqVectorLen, 0.0);
 
   auto buffer_size = buffer.size();
+  if (buffer_size == 0) {
+    return 0.0;
+  }
+
   for (unsigned char chr : buffer) {
     if (chr < kCharFreqVectorLen) {
       buffer_freqs[chr] += 1.0 / buffer_size;
@@ -227,6 +231,10 @@ double WindowsEventLogPublisher::cosineSimilarity(
   mag1 = std::sqrt(mag1);
   mag2 = std::sqrt(mag2);
 
+  if (mag1 * mag2 == 0.0) {
+    return 0.0;
+  }
+
   return dot / (mag1 * mag2);
 }
 

osquery/tables/applications/posix/carbon_black.cpp

@@ -92,7 +92,6 @@ void getSensorSettings(Row& r) {
   boost::replace_all(server, "%3A", ":");
   r["sensor_backend_server"] = SQL_TEXT(server);
   r["collect_data_file_writes"] = INTEGER(0);
-  r["collect_processes"] = INTEGER(0);
   r["collect_sensor_operations"] = INTEGER(0);
   r["log_file_disk_quota_mb"] = INTEGER(0);
   r["log_file_disk_quota_percentage"] = INTEGER(0);
@@ -135,3 +134,4 @@ QueryData genCarbonBlackInfo(QueryContext& context) {
 }
 } // namespace tables
 } // namespace osquery
+

osquery/tables/system/linux/md_tables.cpp

@@ -121,9 +121,11 @@ std::string MD::getPathByDevName(const std::string& name) {
   std::string devPath;
 
   walkUdevDevices("block", [&](udev_device* const& device) {
-    auto const devName = std::string(
-      udev_device_get_property_value(device, "DEVNAME")
-    );
+    const char* devNamePtr = udev_device_get_property_value(device, "DEVNAME");
+    if (devNamePtr == nullptr) {
+      return false;
+    }
+    auto const devName = std::string(devNamePtr);
     if (boost::ends_with(devName, name)) {
       if (!boost::starts_with(devPath, "/")) {
         devPath = "/dev/" + devPath;
@@ -146,8 +148,15 @@ std::string MD::getDevName(int major, int minor) {
     const char* devMajor = udev_device_get_property_value(device, "MAJOR");
     const char* devMinor = udev_device_get_property_value(device, "MINOR");
 
+    if (devMajor == nullptr || devMinor == nullptr) {
+      return false;
+    }
+
     if (std::stoi(devMajor) == major && std::stoi(devMinor) == minor) {
-      devName = udev_device_get_property_value(device, "DEVNAME");
+      const char* name = udev_device_get_property_value(device, "DEVNAME");
+      if (name != nullptr) {
+        devName = name;
+      }
       return true;
     }
 
@@ -163,10 +172,18 @@ std::string MD::getSuperblkVersion(const std::string& arrayName) {
   walkUdevDevices("block", [&](udev_device* const& device) {
     const char* devName = udev_device_get_property_value(device, "DEVNAME");
 
+    if (devName == nullptr) {
+      return false;
+    }
+
     if (arrayName.compare(strlen(devName) - arrayName.length(),
                           std::string::npos,
                           devName) == 0) {
-      version = udev_device_get_property_value(device, "MD_METADATA");
+      const char* metadata =
+          udev_device_get_property_value(device, "MD_METADATA");
+      if (metadata != nullptr) {
+        version = metadata;
+      }
       return true;
     }
 
@@ -785,3 +802,4 @@ QueryData genMDPersonalities(QueryContext& context) {
 }
 } // namespace tables
 } // namespace osquery
+

osquery/utils/pidfile/pidfile_windows.cpp

@@ -146,7 +146,7 @@ Expected<std::string, Pidfile::Error> Pidfile::readFile(
 
   auto remaining_bytes = buffer.size();
 
-  for (int retry = 0; retry < 5; ++retry) {
+  for (int retry = 0; retry < 5 && remaining_bytes > 0; ++retry) {
     auto buffer_ptr = buffer.data() + buffer.size() - remaining_bytes;
 
     DWORD bytes_read{};
@@ -161,6 +161,10 @@ Expected<std::string, Pidfile::Error> Pidfile::readFile(
     remaining_bytes -= static_cast<std::size_t>(bytes_read);
   }
 
+  if (remaining_bytes != 0) {
+    return createError(Pidfile::Error::IOError);
+  }
+
   return buffer;
 }
 
@@ -173,3 +177,4 @@ void Pidfile::destroyFile(FileHandle file_handle, const std::string&) noexcept {
 }
 
 } // namespace osquery
+

osquery/utils/system/uptime.cpp

@@ -22,7 +22,7 @@
 namespace osquery {
 
 long getUptime() {
-#if defined(DARWIN)
+#if defined(__APPLE__)
   struct timeval boot_time;
   size_t len = sizeof(boot_time);
   int mib[2] = {CTL_KERN, KERN_BOOTTIME};
@@ -51,3 +51,4 @@ long getUptime() {
 }
 
 } // namespace osquery
+

plugins/database/rocksdb.cpp

@@ -426,9 +426,6 @@ Status RocksDBDatabasePlugin::removeRange(const std::string& domain,
     options.sync = false;
   }
   auto s = getDB()->DeleteRange(options, cfh, low, high);
-  if (low <= high) {
-    s = getDB()->Delete(options, cfh, high);
-  }
   return Status(s.code(), s.ToString());
 }
 
@@ -466,3 +463,4 @@ Status RocksDBDatabasePlugin::scan(const std::string& domain,
   return Status::success();
 }
 } // namespace osquery
+

plugins/logger/windows_event_log.cpp

@@ -64,7 +64,10 @@ void WindowsEventLoggerPlugin::init(const std::string& name,
     return;
   }
 
-  logStatus(log);
+  auto log_status = logStatus(log);
+  if (!log_status.ok()) {
+    LOG(ERROR) << log_status.getMessage();
+  }
 }
 
 Status WindowsEventLoggerPlugin::acquireHandle(REGHANDLE& registration_handle) {
@@ -141,3 +144,4 @@ Status WindowsEventLoggerPlugin::emitLogRecord(
   return Status();
 }
 } // namespace osquery
+

tests/integration/tables/azure_instance_metadata.cpp

@@ -28,7 +28,8 @@ TEST_F(azureInstanceMetadata, test_sanity) {
         {"architecture", NormalType},
         {"offer", NormalType},
         {"publisher", NormalType},
-        {"sku", NormalType} {"version", NormalType},
+        {"sku", NormalType},
+        {"version", NormalType},
         {"os_type", NormalType},
         {"platform_update_domain", NormalType},
         {"platform_fault_domain", NormalType},
@@ -42,6 +43,7 @@ TEST_F(azureInstanceMetadata, test_sanity) {
     };
     validate_rows(data, row_map);
   }
+}
 
 } // namespace table_tests
-} // namespace table_tests
+} // namespace osquery

tools/tests/test_windows_service.py

@@ -103,11 +103,11 @@ def sc(*args):
             ['sc.exe'] + list(args),
             stderr=subprocess.PIPE,
             stdout=subprocess.PIPE)
-    except subprocess.CalledProcessError, err:
+    except subprocess.CalledProcessError as err:
         return (err.returncode, err.output)
 
     out, _ = p.communicate()
-    out = [x.strip() for x in out.split('\r\n') if x.strip() is not '']
+    out = [x.strip() for x in out.split('\r\n') if x.strip() != '']
 
     if len(out) >= 1:
         if 'SUCCESS' in out[0]:
@@ -246,10 +246,10 @@ def setUp(self):
         self.flagfile = os.path.join(self.tmp_dir, 'osquery.flags')
 
         # Write out our mock configuration files
-        with open(self.config_path, 'wb') as fd:
+        with open(self.config_path, 'w') as fd:
             fd.write(CONFIG_FILE)
 
-        with open(self.flagfile, 'wb') as fd:
+        with open(self.flagfile, 'w') as fd:
             fd.write(
                 FLAGS_FILE.format(self.log_path, self.pidfile,
                                   test_http_server.HTTP_SERVER_CA,