fix(adhoc-sweep-fixes): CU-86akhf8u2 56 review findings across 40 files

osquery/carver/carver.cpp

@@ -280,6 +280,9 @@ Status Carver::blockwiseCopy(PlatformFile& src, PlatformFile& dst) {
       if (bytesWritten < 0) {
         return Status(1, "Error writing bytes to tmp fs");
       }
+      if (bytesWritten < bytesRead) {
+        return Status(1, "Partial write to tmp fs: wrote fewer bytes than read");
+      }
     }
   }
 
@@ -334,6 +337,7 @@ Status Carver::postCarve(const boost::filesystem::path& path) {
   auto contUri = TLSRequestHelper::makeURI(FLAGS_carver_continue_endpoint);
   Request<TLSTransport, JSONSerializer> contRequest(contUri);
   contRequest.setOption("hostname", FLAGS_tls_hostname);
+  size_t failedBlocks = 0;
   for (size_t i = 0; i < blkCount; i++) {
     std::vector<char> block(FLAGS_carver_block_size, 0);
     auto r = pFile.read(block.data(), FLAGS_carver_block_size);
@@ -354,10 +358,18 @@ Status Carver::postCarve(const boost::filesystem::path& path) {
     if (!status.ok()) {
       VLOG(1) << "Post of carved block " << i
               << " failed: " << status.getMessage();
+      failedBlocks++;
       continue;
     }
   }
 
+  if (failedBlocks > 0) {
+    updateCarveValue(carveGuid_, "status", "DATA POST FAILED");
+    return Status(1,
+                  "Failed to post " + std::to_string(failedBlocks) + " of " +
+                      std::to_string(blkCount) + " carve blocks");
+  }
+
   updateCarveValue(carveGuid_, "status", kCarverStatusSuccess);
   return Status::success();
 };

osquery/core/windows/wmi.cpp

@@ -205,7 +205,7 @@ Status WmiResultItem::GetUnsignedInt32(const std::string& name,
     VariantClear(&value);
     return Status::failure("Invalid data type returned.");
   }
-  ret = value.uiVal;
+  ret = value.uintVal;
   VariantClear(&value);
   return Status::success();
 }
@@ -238,7 +238,7 @@ Status WmiResultItem::GetUnsignedLong(const std::string& name,
     VariantClear(&value);
     return Status::failure("Invalid data type returned.");
   }
-  ret = value.lVal;
+  ret = value.ulVal;
   VariantClear(&value);
   return Status::success();
 }
@@ -255,7 +255,7 @@ Status WmiResultItem::GetLongLong(const std::string& name,
     VariantClear(&value);
     return Status::failure("Invalid data type returned.");
   }
-  ret = value.lVal;
+  ret = value.llVal;
   VariantClear(&value);
   return Status::success();
 }
@@ -272,7 +272,7 @@ Status WmiResultItem::GetUnsignedLongLong(const std::string& name,
     VariantClear(&value);
     return Status::failure("Invalid data type returned.");
   }
-  ret = value.lVal;
+  ret = value.ullVal;
   VariantClear(&value);
   return Status::success();
 }
@@ -598,3 +598,4 @@ Status WmiRequest::ExecMethod(const WmiResultItem& object,
 }
 
 } // namespace osquery
+

osquery/database/ephemeral.cpp

@@ -154,7 +154,10 @@ Status EphemeralDatabasePlugin::putBatch(const std::string& domain,
 
 Status EphemeralDatabasePlugin::remove(const std::string& domain,
                                        const std::string& k) {
-  db_[domain].erase(k);
+  auto it = db_.find(domain);
+  if (it != db_.end()) {
+    it->second.erase(k);
+  }
   return Status(0);
 }
 
@@ -165,8 +168,12 @@ Status EphemeralDatabasePlugin::removeRange(const std::string& domain,
     return Status::failure("Invalid range: low > high");
   }
 
+  if (db_.count(domain) == 0) {
+    return Status(0);
+  }
+
   std::vector<std::string> keys;
-  for (const auto& it : db_[domain]) {
+  for (const auto& it : db_.at(domain)) {
     if (it.first >= low && it.first <= high) {
       keys.push_back(it.first);
     }

osquery/events/darwin/es_utils.cpp

@@ -157,9 +157,9 @@ void getProcessProperties(const es_process_t* p,
   ec->codesigning_flags = getCodesigningFlags(p);
 
   auto user = getpwuid(ec->uid);
-  ec->username = user->pw_name != nullptr ? std::string(user->pw_name) : "";
-
-  ec->cwd = getCwdPathFromPid(ec->pid);
+  ec->username = (user != nullptr && user->pw_name != nullptr)
+                     ? std::string(user->pw_name)
+                     : "";
 }
 
 void appendQuotedString(std::ostream& out, std::string s, char delim) {
@@ -171,3 +171,4 @@ void appendQuotedString(std::ostream& out, std::string s, char delim) {
 }
 
 } // namespace osquery
+

osquery/events/darwin/fsevents.cpp

@@ -233,9 +233,6 @@ void FSEventsEventPublisher::configure() {
     paths_.clear();
     for (auto& sub : subscriptions_) {
       auto sc = getSubscriptionContext(sub->context);
-      if (sc->discovered_.size() > 0) {
-        continue;
-      }
       auto paths = transformSubscription(sc);
       paths_.insert(paths.begin(), paths.end());
     }

osquery/events/darwin/scnetwork.cpp

@@ -50,7 +50,7 @@ void SCNetworkEventPublisher::addTarget(
 
   // Assign a context (the subscription context) to the target.
   SCNetworkReachabilityContext* context = new SCNetworkReachabilityContext();
-  context->info = (void*)≻
+  context->info = (void*)(new SCNetworkSubscriptionContextRef(sc));
   context->retain = nullptr;
   context->release = nullptr;
   contexts_.push_back(context);
@@ -94,6 +94,7 @@ void SCNetworkEventPublisher::clearAll() {
   targets_.clear();
 
   for (auto& context : contexts_) {
+    delete (SCNetworkSubscriptionContextRef*)(context->info);
     delete context;
   }
   contexts_.clear();
@@ -116,14 +117,14 @@ void SCNetworkEventPublisher::configure() {
       if (sc->type == ADDRESS_TARGET) {
         auto existing_address = std::find(
             target_addresses_.begin(), target_addresses_.end(), sc->target);
-        if (existing_address != target_addresses_.end()) {
+        if (existing_address == target_addresses_.end()) {
           // Add the address target.
           addAddress(sc);
         }
       } else {
         auto existing_hostname =
             std::find(target_names_.begin(), target_names_.end(), sc->target);
-        if (existing_hostname != target_names_.end()) {
+        if (existing_hostname == target_names_.end()) {
           // Add the hostname target.
           addHostname(sc);
         }
@@ -183,3 +184,4 @@ Status SCNetworkEventPublisher::run() {
   return Status::success();
 }
 };
+

osquery/events/events.cpp

@@ -40,7 +40,9 @@ bool enforceEventsDenylist(const std::string& query) {
   // Check if the query only operates on event subscribers.
   // If it does, skip the denylist enforcement.
   std::set<std::string> table_set(tables.begin(), tables.end());
-  auto event_tables = EventFactory::subscriberNames();
+  auto subscriber_names = EventFactory::subscriberNames();
+  std::set<std::string> event_tables(subscriber_names.begin(),
+                                     subscriber_names.end());
 
   std::set<std::string> overlap;
   std::set_intersection(table_set.begin(),

osquery/events/linux/bpf/filesystem.cpp

@@ -128,7 +128,14 @@ bool Filesystem::enumFiles(int dirfd, EnumFilesCallback callback) const {
     bool directory;
     if (entry->d_type == DT_DIR) {
       directory = true;
-    } else if (entry->d_type == DT_LNK || entry->d_type == DT_REG) {
+    } else if (entry->d_type == DT_LNK) {
+      struct stat file_stats {};
+      if (fstatat(dirfd, string_fd, &file_stats, 0) != 0) {
+        continue;
+      }
+
+      directory = S_ISDIR(file_stats.st_mode);
+    } else if (entry->d_type == DT_REG) {
       directory = false;
     } else {
       continue;
@@ -171,3 +178,4 @@ Status IFilesystem::create(Ref& obj) {
 }
 
 } // namespace osquery
+

osquery/events/linux/inotify.cpp

@@ -67,6 +67,10 @@ Status INotifyEventPublisher::setUp() {
   }
 
   WriteLock lock(scratch_mutex_);
+  if (scratch_ != nullptr) {
+    free(scratch_);
+    scratch_ = nullptr;
+  }
   scratch_ = (char*)malloc(kINotifyBufferSize);
   if (scratch_ == nullptr) {
     return Status(1, "Could not allocate scratch space");
@@ -351,7 +355,8 @@ bool INotifyEventPublisher::shouldFire(const INotifySubscriptionContextRef& sc,
   // Need to have two finds,
   // what if somebody excluded an individual file inside a directory
   if (!exclude_paths_.empty() &&
-      (exclude_paths_.find(path) || exclude_paths_.find(ec->path))) {
+      (exclude_paths_.find(path) != exclude_paths_.end() ||
+       exclude_paths_.find(ec->path) != exclude_paths_.end())) {
     return false;
   }
 
@@ -486,3 +491,4 @@ bool INotifyEventPublisher::isPathMonitored(const std::string& path) const {
   return (path_iterator != path_descriptors_.end());
 }
 }
+

osquery/events/windows/etw/etw_kernel_session.cpp

@@ -136,7 +136,9 @@ void KernelEtwSessionRunnable::start() {
   std::unique_lock<std::mutex> lock(mutex_);
   if (kernelTraceSession_) {
     while (!endTraceSession_) {
+      lock.unlock();
       kernelTraceSession_->start();
+      lock.lock();
       traceSessionStopped_ = true;
 
       if (!endTraceSession_) {
@@ -156,15 +158,19 @@ void KernelEtwSessionRunnable::stop() {
 void KernelEtwSessionRunnable::pause() {
   if (kernelTraceSession_) {
     kernelTraceSession_->stop();
+    std::unique_lock<std::mutex> lock(mutex_);
     while (!traceSessionStopped_) {
+      lock.unlock();
       Sleep(500);
+      lock.lock();
     }
     traceSessionStopped_ = false;
   }
 }
 
 void KernelEtwSessionRunnable::resume() {
   if (kernelTraceSession_) {
+    std::unique_lock<std::mutex> lock(mutex_);
     condition_.notify_one();
   }
 }

osquery/events/windows/etw/etw_provider_config.cpp

@@ -26,8 +26,8 @@ Status EtwProviderConfig::isValid() const {
     return Status::failure("Type handlers were not provided");
   }
 
-  if (getPostProcessor() == nullptr) {
-    return Status::failure("Invalid Provider PostProcessor function");
+  if (getPreProcessor() == nullptr) {
+    return Status::failure("Invalid Provider PreProcessor function");
   }
 
   return Status::success();
@@ -166,4 +166,4 @@ void EtwProviderConfig::addEventTypeToHandle(const EtwEventType& value) {
   eventTypes_.push_back(value);
 }
 
-} // namespace osquery
+} // namespace osquery

osquery/filesystem/file_compression.cpp

@@ -48,8 +48,8 @@ Status compress(const boost::filesystem::path& in,
 
   size_t const buffInSize = ZSTD_CStreamInSize();
   size_t const buffOutSize = ZSTD_CStreamOutSize();
-  std::vector<void*> buffIn(buffInSize);
-  std::vector<void*> buffOut(buffOutSize);
+  std::vector<unsigned char> buffIn(buffInSize);
+  std::vector<unsigned char> buffOut(buffOutSize);
   auto read = buffInSize;
   auto toRead = buffInSize;
   size_t readSoFar = 0;
@@ -114,8 +114,8 @@ Status decompress(const boost::filesystem::path& in,
   auto inFileSize = inFile.size();
   size_t const buffInSize = ZSTD_DStreamInSize();
   size_t const buffOutSize = ZSTD_DStreamOutSize();
-  std::vector<void*> buffIn(buffInSize);
-  std::vector<void*> buffOut(buffOutSize);
+  std::vector<unsigned char> buffIn(buffInSize);
+  std::vector<unsigned char> buffOut(buffOutSize);
 
   ZSTD_DStream* const dstream = ZSTD_createDStream();
   if (dstream == NULL) {
@@ -201,3 +201,4 @@ Status archive(const std::set<boost::filesystem::path>& paths,
   return Status::success();
 };
 } // namespace osquery
+

osquery/sql/dynamic_table_row.cpp

@@ -101,20 +101,22 @@ int DynamicTableRow::get_column(sqlite3_context* ctx,
   }
 
   // Attempt to cast each xFilter-populated row/column to the SQLite type.
-  const auto& value = row[column_name];
-  if (this->row.count(column_name) == 0) {
+  auto row_it = this->row.find(column_name);
+  if (row_it == this->row.end()) {
     // Missing content.
     VLOG(1) << "Error " << column_name << " is empty";
     sqlite3_result_null(ctx);
-  } else if (type == TEXT_TYPE || type == BLOB_TYPE) {
+  } else if (const auto& value = row_it->second;
+             type == TEXT_TYPE || type == BLOB_TYPE) {
     sqlite3_result_text(
         ctx, value.c_str(), static_cast<int>(value.size()), SQLITE_TRANSIENT);
-  } else if (value.empty() &&
+  } else if (const auto& value = row_it->second;
+             value.empty() &&
              (type == INTEGER_TYPE || type == BIGINT_TYPE ||
               type == UNSIGNED_BIGINT_TYPE || type == DOUBLE_TYPE)) {
     // Don't Log a casting error for a known type if the column row is empty
     sqlite3_result_null(ctx);
-  } else if (type == INTEGER_TYPE) {
+  } else if (const auto& value = row_it->second; type == INTEGER_TYPE) {
     auto afinite = tryTo<long>(value, 0);
     if (afinite.isError()) {
       VLOG(1) << "Error casting " << column_name << " (" << value
@@ -123,7 +125,8 @@ int DynamicTableRow::get_column(sqlite3_context* ctx,
     } else {
       sqlite3_result_int(ctx, afinite.take());
     }
-  } else if (type == BIGINT_TYPE || type == UNSIGNED_BIGINT_TYPE) {
+  } else if (const auto& value = row_it->second;
+             type == BIGINT_TYPE || type == UNSIGNED_BIGINT_TYPE) {
     auto afinite = tryTo<long long>(value, 0);
     if (afinite.isError()) {
       VLOG(1) << "Error casting " << column_name << " (" << value
@@ -132,7 +135,7 @@ int DynamicTableRow::get_column(sqlite3_context* ctx,
     } else {
       sqlite3_result_int64(ctx, afinite.take());
     }
-  } else if (type == DOUBLE_TYPE) {
+  } else if (const auto& value = row_it->second; type == DOUBLE_TYPE) {
     char* end = nullptr;
     double afinite = strtod(value.c_str(), &end);
     if (end == nullptr || end == value.c_str() || *end != '\0') {
@@ -163,3 +166,4 @@ TableRowHolder DynamicTableRow::clone() const {
 }
 
 } // namespace osquery
+