Fix string length issues (#1070)

Source/common/BUILD

@@ -69,6 +69,11 @@ objc_library(
     hdrs = ["Platform.h"],
 )
 
+objc_library(
+    name = "String",
+    hdrs = ["String.h"],
+)
+
 objc_library(
     name = "SantaVnodeHash",
     srcs = ["SantaVnodeHash.mm"],

Source/common/String.h

@@ -0,0 +1,35 @@
+/// Copyright 2023 Google LLC
+///
+/// Licensed under the Apache License, Version 2.0 (the "License");
+/// you may not use this file except in compliance with the License.
+/// You may obtain a copy of the License at
+///
+///     https://www.apache.org/licenses/LICENSE-2.0
+///
+/// Unless required by applicable law or agreed to in writing, software
+/// distributed under the License is distributed on an "AS IS" BASIS,
+/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+/// See the License for the specific language governing permissions and
+/// limitations under the License.
+
+#ifndef SANTA__COMMON__STRING_H
+#define SANTA__COMMON__STRING_H
+
+#include <Foundation/Foundation.h>
+
+#include <string>
+#include <string_view>
+
+namespace santa::common {
+
+static inline std::string_view NSStringToUTF8StringView(NSString *str) {
+  return std::string_view(str.UTF8String, [str lengthOfBytesUsingEncoding:NSUTF8StringEncoding]);
+}
+
+static inline std::string NSStringToUTF8String(NSString *str) {
+  return std::string(str.UTF8String, [str lengthOfBytesUsingEncoding:NSUTF8StringEncoding]);
+}
+
+}  // namespace santa::common
+
+#endif

Source/santad/BUILD

@@ -49,6 +49,7 @@ objc_library(
         ":WatchItemPolicy",
         "//Source/common:PrefixTree",
         "//Source/common:SNTLogging",
+        "//Source/common:String",
         "//Source/common:Unit",
     ],
 )
@@ -232,6 +233,7 @@ objc_library(
         "//Source/common:SNTRule",
         "//Source/common:SNTStoredEvent",
         "//Source/common:SantaVnode",
+        "//Source/common:String",
         "@MOLCodesignChecker",
     ],
 )
@@ -468,6 +470,7 @@ objc_library(
     ],
     deps = [
         ":EndpointSecurityMessage",
+        "//Source/common:String",
     ],
 )
 
@@ -501,6 +504,7 @@ objc_library(
         "//Source/common:SNTConfigurator",
         "//Source/common:SNTLogging",
         "//Source/common:SNTStoredEvent",
+        "//Source/common:String",
         "//Source/common:santa_cc_proto_library_wrapper",
     ],
 )

Source/santad/DataLayer/WatchItems.mm

@@ -34,8 +34,12 @@
 
 #import "Source/common/PrefixTree.h"
 #import "Source/common/SNTLogging.h"
+#import "Source/common/String.h"
 #import "Source/common/Unit.h"
 #include "Source/santad/DataLayer/WatchItemPolicy.h"
+
+using santa::common::NSStringToUTF8String;
+using santa::common::NSStringToUTF8StringView;
 using santa::common::PrefixTree;
 using santa::common::Unit;
 using santa::santad::data_layer::WatchItemPathType;
@@ -255,7 +259,7 @@ std::variant<Unit, PathList> VerifyConfigWatchItemPaths(NSArray<id> *paths, NSEr
         return Unit{};
       }
 
-      path_list.push_back({std::string(path_str.UTF8String, path_str.length), path_type});
+      path_list.push_back({NSStringToUTF8String(path_str), path_type});
     } else if ([path isKindOfClass:[NSString class]]) {
       if (!LenRangeValidator(1, PATH_MAX)(path, err)) {
         PopulateError(err, [NSString stringWithFormat:@"Invalid path length: %@",
@@ -264,8 +268,8 @@ std::variant<Unit, PathList> VerifyConfigWatchItemPaths(NSArray<id> *paths, NSEr
         return Unit{};
       }
 
-      path_list.push_back({std::string(((NSString *)path).UTF8String, ((NSString *)path).length),
+      path_list.push_back(
-                           kWatchItemPolicyDefaultPathType});
+        {NSStringToUTF8String(((NSString *)path)), kWatchItemPolicyDefaultPathType});
     } else {
       PopulateError(
         err, [NSString stringWithFormat:
@@ -340,12 +344,11 @@ std::variant<Unit, ProcessList> VerifyConfigWatchItemProcesses(NSDictionary *wat
           }
 
           proc_list.push_back(WatchItemPolicy::Process(
-            std::string([(process[kWatchItemConfigKeyProcessesBinaryPath] ?: @"") UTF8String]),
+            NSStringToUTF8String(process[kWatchItemConfigKeyProcessesBinaryPath] ?: @""),
-            std::string([(process[kWatchItemConfigKeyProcessesSigningID] ?: @"") UTF8String]),
+            NSStringToUTF8String(process[kWatchItemConfigKeyProcessesSigningID] ?: @""),
-            std::string([(process[kWatchItemConfigKeyProcessesTeamID] ?: @"") UTF8String]),
+            NSStringToUTF8String(process[kWatchItemConfigKeyProcessesTeamID] ?: @""),
             HexStringToBytes(process[kWatchItemConfigKeyProcessesCDHash]),
-            std::string(
+            NSStringToUTF8String(process[kWatchItemConfigKeyProcessesCertificateSha256] ?: @""),
-              [(process[kWatchItemConfigKeyProcessesCertificateSha256] ?: @"") UTF8String]),
             process[kWatchItemConfigKeyProcessesPlatformBinary]
               ? std::make_optional(
                   (bool)[process[kWatchItemConfigKeyProcessesPlatformBinary] boolValue])
@@ -423,8 +426,8 @@ bool ParseConfigSingleWatchItem(NSString *name, NSDictionary *watch_item,
 
   for (const PathAndTypePair &path_type_pair : std::get<PathList>(path_list)) {
     policies.push_back(std::make_shared<WatchItemPolicy>(
-      [name UTF8String], path_type_pair.first, path_type_pair.second, allow_read_access, audit_only,
+      NSStringToUTF8StringView(name), path_type_pair.first, path_type_pair.second,
-      std::get<ProcessList>(proc_list)));
+      allow_read_access, audit_only, std::get<ProcessList>(proc_list)));
   }
 
   return true;
@@ -645,7 +648,7 @@ void WatchItems::UpdateCurrentState(
     std::swap(currently_monitored_paths_, new_monitored_paths);
     current_config_ = new_config;
     if (new_config) {
-      policy_version_ = [new_config[kWatchItemConfigKeyVersion] UTF8String];
+      policy_version_ = NSStringToUTF8String(new_config[kWatchItemConfigKeyVersion]);
     } else {
       policy_version_ = "";
     }

Source/santad/Logs/EndpointSecurity/Serializers/Protobuf.mm

@@ -23,6 +23,7 @@
 #include <sys/wait.h>
 #include <time.h>
 
+#include <functional>
 #include <optional>
 #include <string_view>
 
@@ -30,6 +31,7 @@
 #import "Source/common/SNTConfigurator.h"
 #include "Source/common/SNTLogging.h"
 #import "Source/common/SNTStoredEvent.h"
+#import "Source/common/String.h"
 #include "Source/santad/EventProviders/EndpointSecurity/EndpointSecurityAPI.h"
 #include "Source/santad/Logs/EndpointSecurity/Serializers/Utilities.h"
 #import "Source/santad/SNTDecisionCache.h"
@@ -38,6 +40,7 @@
 using google::protobuf::Arena;
 using google::protobuf::Timestamp;
 
+using santa::common::NSStringToUTF8StringView;
 using santa::santad::event_providers::endpoint_security::EndpointSecurityAPI;
 using santa::santad::event_providers::endpoint_security::EnrichedClose;
 using santa::santad::event_providers::endpoint_security::EnrichedEventType;
@@ -94,15 +97,15 @@ static inline void EncodePath(std::string *buf, const es_file_t *es_file) {
   buf->append(std::string_view(es_file->path.data, es_file->path.length));
 }
 
-static inline void EncodeString(std::string *buf, NSString *value) {
+static inline void EncodeString(std::function<std::string *()> lazy_f, NSString *value) {
   if (value) {
-    buf->append(std::string_view([value UTF8String], [value length]));
+    lazy_f()->append(NSStringToUTF8StringView(value));
   }
 }
 
-static inline void EncodeString(std::string *buf, std::string_view value) {
+static inline void EncodeString(std::function<std::string *()> lazy_f, std::string_view value) {
   if (value.length() > 0) {
-    buf->append(std::string_view(value.data(), value.length()));
+    lazy_f()->append(value);
   }
 }
 
@@ -125,7 +128,7 @@ static inline void EncodeGroupInfo(::pbv1::GroupInfo *pb_group_info, gid_t gid,
 static inline void EncodeHash(::pbv1::Hash *pb_hash, NSString *sha256) {
   if (sha256) {
     pb_hash->set_type(::pbv1::Hash::HASH_ALGO_SHA256);
-    pb_hash->set_hash([sha256 UTF8String], [sha256 length]);
+    EncodeString([pb_hash] { return pb_hash->mutable_hash(); }, sha256);
   }
 }
 
@@ -162,7 +165,7 @@ static inline void EncodeFileInfo(::pbv1::FileInfo *pb_file, const es_file_t *es
 
 static inline void EncodeFileInfoLight(::pbv1::FileInfoLight *pb_file, std::string_view path,
                                        bool truncated) {
-  EncodeString(pb_file->mutable_path(), path);
+  EncodeString([pb_file] { return pb_file->mutable_path(); }, path);
   pb_file->set_truncated(truncated);
 }
 
@@ -262,9 +265,7 @@ static inline void EncodeCertificateInfo(::pbv1::CertificateInfo *pb_cert_info,
     EncodeHash(pb_cert_info->mutable_hash(), cert_hash);
   }
 
-  if (common_name) {
+  EncodeString([pb_cert_info] { return pb_cert_info->mutable_common_name(); }, common_name);
-    pb_cert_info->set_common_name([common_name UTF8String], [common_name length]);
-  }
 }
 
 ::pbv1::Execution::Decision GetDecisionEnum(SNTEventState event_state) {
@@ -356,7 +357,7 @@ static inline void EncodeCertificateInfo(::pbv1::CertificateInfo *pb_cert_info,
   ::pbv1::SantaMessage *santa_msg = Arena::CreateMessage<::pbv1::SantaMessage>(arena);
 
   if (EnabledMachineID()) {
-    EncodeString(santa_msg->mutable_machine_id(), MachineID());
+    EncodeString([santa_msg] { return santa_msg->mutable_machine_id(); }, MachineID());
   }
   EncodeTimestamp(santa_msg->mutable_event_time(), event_time);
   EncodeTimestamp(santa_msg->mutable_processed_time(), processed_time);
@@ -491,18 +492,11 @@ std::vector<uint8_t> Protobuf::SerializeMessage(const EnrichedExec &msg, SNTCach
     EncodeCertificateInfo(pb_exec->mutable_certificate_info(), cd.certSHA256, cd.certCommonName);
   }
 
-  if (cd.decisionExtra) {
+  EncodeString([pb_exec] { return pb_exec->mutable_explain(); }, cd.decisionExtra);
-    pb_exec->set_explain([cd.decisionExtra UTF8String], [cd.decisionExtra length]);
+  EncodeString([pb_exec] { return pb_exec->mutable_quarantine_url(); }, cd.quarantineURL);
-  }
-
-  if (cd.quarantineURL) {
-    pb_exec->set_quarantine_url([cd.quarantineURL UTF8String], [cd.quarantineURL length]);
-  }
 
   NSString *orig_path = Utilities::OriginalPathForTranslocation(msg.es_msg().event.exec.target);
-  if (orig_path) {
+  EncodeString([pb_exec] { return pb_exec->mutable_original_path(); }, orig_path);
-    pb_exec->set_original_path([orig_path UTF8String], [orig_path length]);
-  }
 
   return FinalizeProto(santa_msg);
 }
@@ -594,8 +588,9 @@ std::vector<uint8_t> Protobuf::SerializeFileAccess(const std::string &policy_ver
   EncodeProcessInfo(file_access->mutable_instigator(), msg->version, msg->process,
                     enriched_process);
   EncodeFileInfoLight(file_access->mutable_target(), target, false);
-  EncodeString(file_access->mutable_policy_version(), policy_version);
+  EncodeString([file_access] { return file_access->mutable_policy_version(); }, policy_version);
-  EncodeString(file_access->mutable_policy_name(), policy_name);
+  EncodeString([file_access] { return file_access->mutable_policy_name(); }, policy_name);
+
   file_access->set_access_type(GetAccessType(msg->event_type));
   file_access->set_policy_decision(GetPolicyDecision(decision));
 
@@ -629,10 +624,12 @@ std::vector<uint8_t> Protobuf::SerializeBundleHashingEvent(SNTStoredEvent *event
 
   EncodeHash(pb_bundle->mutable_file_hash(), event.fileSHA256);
   EncodeHash(pb_bundle->mutable_bundle_hash(), event.fileBundleHash);
-  pb_bundle->set_bundle_name([NonNull(event.fileBundleName) UTF8String]);
+  EncodeString([pb_bundle] { return pb_bundle->mutable_bundle_name(); },
-  pb_bundle->set_bundle_id([NonNull(event.fileBundleID) UTF8String]);
+               NonNull(event.fileBundleName));
-  pb_bundle->set_bundle_path([NonNull(event.fileBundlePath) UTF8String]);
+  EncodeString([pb_bundle] { return pb_bundle->mutable_bundle_id(); }, NonNull(event.fileBundleID));
-  pb_bundle->set_path([NonNull(event.filePath) UTF8String]);
+  EncodeString([pb_bundle] { return pb_bundle->mutable_bundle_path(); },
+               NonNull(event.fileBundlePath));
+  EncodeString([pb_bundle] { return pb_bundle->mutable_path(); }, NonNull(event.filePath));
 
   return FinalizeProto(santa_msg);
 }
@@ -652,14 +649,14 @@ static void EncodeDisk(::pbv1::Disk *pb_disk, ::pbv1::Disk_Action action, NSDict
     stringWithFormat:@"%@ %@", NonNull(props[@"DADeviceVendor"]), NonNull(props[@"DADeviceModel"])];
   model = [model stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceCharacterSet]];
 
-  EncodeString(pb_disk->mutable_mount(), [props[@"DAVolumePath"] path]);
+  EncodeString([pb_disk] { return pb_disk->mutable_mount(); }, [props[@"DAVolumePath"] path]);
-  EncodeString(pb_disk->mutable_volume(), props[@"DAVolumeName"]);
+  EncodeString([pb_disk] { return pb_disk->mutable_volume(); }, props[@"DAVolumeName"]);
-  EncodeString(pb_disk->mutable_bsd_name(), props[@"DAMediaBSDName"]);
+  EncodeString([pb_disk] { return pb_disk->mutable_bsd_name(); }, props[@"DAMediaBSDName"]);
-  EncodeString(pb_disk->mutable_fs(), props[@"DAVolumeKind"]);
+  EncodeString([pb_disk] { return pb_disk->mutable_fs(); }, props[@"DAVolumeKind"]);
-  EncodeString(pb_disk->mutable_model(), model);
+  EncodeString([pb_disk] { return pb_disk->mutable_model(); }, model);
-  EncodeString(pb_disk->mutable_serial(), serial);
+  EncodeString([pb_disk] { return pb_disk->mutable_serial(); }, serial);
-  EncodeString(pb_disk->mutable_bus(), props[@"DADeviceProtocol"]);
+  EncodeString([pb_disk] { return pb_disk->mutable_bus(); }, props[@"DADeviceProtocol"]);
-  EncodeString(pb_disk->mutable_dmg_path(), dmg_path);
+  EncodeString([pb_disk] { return pb_disk->mutable_dmg_path(); }, dmg_path);
 
   if (props[@"DAAppearanceTime"]) {
     // Note: `DAAppearanceTime` is set via `CFAbsoluteTimeGetCurrent`, which uses the defined

Source/santad/Logs/EndpointSecurity/Serializers/SanitizableString.h

@@ -51,8 +51,7 @@ class SanitizableString {
   friend std::ostream &operator<<(std::ostream &ss, const SanitizableString &sani_string);
 
  private:
-  const char *data_;
+  std::string_view data_;
-  size_t length_;
   mutable bool sanitized_ = false;
   mutable std::optional<std::string> sanitized_string_;
 };

Source/santad/Logs/EndpointSecurity/Serializers/SanitizableString.mm

@@ -14,37 +14,35 @@
 
 #include "Source/santad/Logs/EndpointSecurity/Serializers/SanitizableString.h"
 
+#include "Source/common/String.h"
+
+using santa::common::NSStringToUTF8StringView;
+
 namespace santa::santad::logs::endpoint_security::serializers {
 
 SanitizableString::SanitizableString(const es_file_t *file)
-    : data_(file->path.data), length_(file->path.length) {}
+    : data_(file->path.data, file->path.length) {}
 
-SanitizableString::SanitizableString(const es_string_token_t &tok)
+SanitizableString::SanitizableString(const es_string_token_t &tok) : data_(tok.data, tok.length) {}
-    : data_(tok.data), length_(tok.length) {}
 
-SanitizableString::SanitizableString(NSString *str)
+SanitizableString::SanitizableString(NSString *str) : data_(NSStringToUTF8StringView(str)) {}
-    : data_([str UTF8String]), length_([str length]) {}
 
-SanitizableString::SanitizableString(const char *str, size_t len) : data_(str), length_(len) {}
+SanitizableString::SanitizableString(const char *str, size_t len) : data_(str, len) {}
 
 std::string_view SanitizableString::String() const {
-  return std::string_view(data_, length_);
+  return data_;
 }
 
 std::string_view SanitizableString::Sanitized() const {
   if (!sanitized_) {
     sanitized_ = true;
-    sanitized_string_ = SanitizeString(data_, length_);
+    sanitized_string_ = SanitizeString(data_.data(), data_.length());
   }
 
   if (sanitized_string_.has_value()) {
     return sanitized_string_.value();
   } else {
-    if (data_) {
+    return data_;
-      return std::string_view(data_, length_);
-    } else {
-      return "";
-    }
   }
 }
 

Source/santad/SNTExecutionController.mm

@@ -36,6 +36,7 @@
 #import "Source/common/SNTRule.h"
 #import "Source/common/SNTStoredEvent.h"
 #include "Source/common/SantaVnode.h"
+#include "Source/common/String.h"
 #import "Source/santad/DataLayer/SNTEventTable.h"
 #import "Source/santad/DataLayer/SNTRuleTable.h"
 #import "Source/santad/SNTDecisionCache.h"
@@ -360,7 +361,8 @@ static NSString *const kPrinterProxyPostMonterey =
 
 - (void)printMessage:(NSString *)msg toTTY:(const char *)path {
   int fd = open(path, O_WRONLY | O_NOCTTY);
-  write(fd, msg.UTF8String, msg.length);
+  std::string_view str = santa::common::NSStringToUTF8StringView(msg);
+  write(fd, str.data(), str.length());
   close(fd);
 }