unicode: Improve Windows unicode conversion functions

osquery/filesystem/windows/fileops.cpp

@@ -117,7 +117,7 @@ Status windowsShortPathToLongPath(const std::string& shortPath,
   if (ret == 0) {
     return Status(GetLastError(), "Failed to convert short path to long path");
   }
-  rLongPath = wstringToString(longPath);
+  rLongPath = wstringToString(longPath, ARRAYSIZE(longPath));
   return Status::success();
 }
 
@@ -131,8 +131,7 @@ Status windowsGetVersionInfo(const std::string& path,
   if (verInfo == nullptr) {
     return Status(1, "Failed to malloc for version info");
   }
-  auto err = GetFileVersionInfoW(
-      stringToWstring(path).c_str(), handle, verSize, verInfo.get());
+  auto err = GetFileVersionInfoW(wpath.c_str(), handle, verSize, verInfo.get());
   if (err == 0) {
     return Status(GetLastError(), "Failed to get file version info");
   }
@@ -1587,7 +1586,7 @@ boost::optional<std::string> getHomeDirectory() {
     return value;
   } else if (SUCCEEDED(::SHGetFolderPathW(
                  nullptr, CSIDL_PROFILE, nullptr, 0, profile.data()))) {
-    return wstringToString(profile.data());
+    return wstringToString(profile.data(), profile.size());
   } else {
     return boost::none;
   }
@@ -1778,12 +1777,15 @@ Status platformStat(const fs::path& path, WINDOWS_STAT* wfile_stat) {
   // std::string, in which case, internal fs::path conversion to wstring will be
   // performed incorrectly.
 
-  if (PathIsDirectoryW(stringToWstring(path.string()).c_str())) {
+  std::string path_str = path.string();
+  std::wstring path_wstr = stringToWstring(path_str);
+
+  if (PathIsDirectoryW(path_wstr.c_str())) {
     FLAGS_AND_ATTRIBUTES |= FILE_FLAG_BACKUP_SEMANTICS;
   }
 
   // Get the handle of the file object.
-  auto file_handle = CreateFileW(stringToWstring(path.string()).c_str(),
+  auto file_handle = CreateFileW(path_wstr.c_str(),
                                  GENERIC_READ,
                                  FILE_SHARE_READ | FILE_SHARE_WRITE,
                                  nullptr,
@@ -1795,7 +1797,7 @@ Status platformStat(const fs::path& path, WINDOWS_STAT* wfile_stat) {
   if (file_handle == INVALID_HANDLE_VALUE) {
     CloseHandle(file_handle);
     return Status(-1,
-                  "CreateFile failed for " + path.string() + " with " +
+                  "CreateFile failed for " + path_str + " with " +
                       std::to_string(GetLastError()));
   }
 
@@ -1817,7 +1819,7 @@ Status platformStat(const fs::path& path, WINDOWS_STAT* wfile_stat) {
   if (ret != ERROR_SUCCESS) {
     CloseHandle(file_handle);
     return Status(-1,
-                  "GetSecurityInfo failed for " + path.string() + " with " +
+                  "GetSecurityInfo failed for " + path_str + " with " +
                       std::to_string(GetLastError()));
   }
 
@@ -1828,7 +1830,7 @@ Status platformStat(const fs::path& path, WINDOWS_STAT* wfile_stat) {
     CloseHandle(file_handle);
     LocalFree(security_descriptor);
     return Status(-1,
-                  "GetFileInformationByHandle failed for " + path.string() +
+                  "GetFileInformationByHandle failed for " + path_str +
                       " with " + std::to_string(GetLastError()));
   }
 
@@ -1908,7 +1910,7 @@ Status platformStat(const fs::path& path, WINDOWS_STAT* wfile_stat) {
                                          : wfile_stat->size = li.QuadPart;
 
   const char* drive_letter = nullptr;
-  auto drive_letter_index = PathGetDriveNumberW(path.wstring().c_str());
+  auto drive_letter_index = PathGetDriveNumberW(path_wstr.c_str());
 
   if (drive_letter_index != -1 && kDriveLetters.count(drive_letter_index)) {
     drive_letter = kDriveLetters.at(drive_letter_index).c_str();
@@ -1944,12 +1946,10 @@ Status platformStat(const fs::path& path, WINDOWS_STAT* wfile_stat) {
   (!ret) ? wfile_stat->ctime = -1
          : wfile_stat->ctime = longIntToUnixtime(basic_info.ChangeTime);
 
-  windowsGetVersionInfo(wstringToString(path.wstring()),
-                        wfile_stat->product_version,
-                        wfile_stat->file_version);
+  windowsGetVersionInfo(
+      path_str, wfile_stat->product_version, wfile_stat->file_version);
 
-  windowsGetOriginalFilename(wstringToString(path.wstring()),
-                             wfile_stat->original_filename);
+  windowsGetOriginalFilename(path_str, wfile_stat->original_filename);
 
   CloseHandle(file_handle);
 

osquery/process/windows/process.cpp

@@ -292,7 +292,7 @@ std::shared_ptr<PlatformProcess> PlatformProcess::launchExtension(
   // of the string content. Also it is guaranteed that the buffer is
   // null-terminated. See
   // https://en.cppreference.com/w/cpp/string/basic_string/data
-  auto argv = argv_stream.str();
+  auto argv_str = argv_stream.str();
 
   // In POSIX, this environment variable is set to the child's process ID. But
   // that is not easily accomplishable on Windows and provides no value since
@@ -305,11 +305,11 @@ std::shared_ptr<PlatformProcess> PlatformProcess::launchExtension(
 
   // We are autoloading a Python extension, so pass off to our helper
   if (ext_path.extension().wstring() == L".ext") {
-    return launchTestPythonScript(wstringToString(argv));
+    return launchTestPythonScript(wstringToString(argv_str));
   } else {
     auto status =
         ::CreateProcess(nullptr,
-                        argv.data(),
+                        argv_str.data(),
                         nullptr,
                         nullptr,
                         TRUE,

osquery/system/usersgroups/windows/users_service.cpp

@@ -105,20 +105,18 @@ std::optional<std::vector<std::string>> getRoamingProfileSids() {
     return {};
   }
 
-  std::wstring key_name;
-  key_name.resize(max_key_length);
+  WCHAR key_name[max_key_length]{};
 
   std::vector<std::string> subkeys_names;
 
   // Process registry subkeys
   for (DWORD i = 0; i < subkeys_count; i++) {
-    ret_code =
-        RegEnumKeyW(registry_handle.get(), i, key_name.data(), max_key_length);
+    ret_code = RegEnumKeyW(registry_handle.get(), i, key_name, max_key_length);
     if (ret_code != ERROR_SUCCESS) {
       return std::nullopt;
     }
 
-    subkeys_names.emplace_back(wstringToString(key_name));
+    subkeys_names.emplace_back(wstringToString(key_name, max_key_length));
   }
 
   return subkeys_names;
@@ -312,7 +310,8 @@ void UsersService::processRoamingProfiles(
       LocalFree(sid);
 
       if (ret != FALSE) {
-        new_user.username = wstringToString(account_name);
+        new_user.username =
+            wstringToString(account_name, ARRAYSIZE(account_name));
         /* NOTE: This still keeps the old behavior where if getting the gid
         from the first local group or the primary group id fails,
         then we use the uid of the user. */

osquery/tables/system/windows/authenticode.cpp

@@ -22,10 +22,10 @@
 #include <iomanip>
 // clang-format on
 
+#include <osquery/core/tables.h>
 #include <osquery/filesystem/filesystem.h>
 #include <osquery/logger/logger.h>
 #include <osquery/sql/sql.h>
-#include <osquery/core/tables.h>
 #include <osquery/utils/conversions/tryto.h>
 #include <osquery/utils/conversions/windows/strings.h>
 
@@ -342,7 +342,7 @@ Status getCertificateInformation(SignatureInformation& signature_info,
       return false;
     }
 
-    value = wstringToString(buffer);
+    value = wstringToString(buffer.data(), buffer.size());
     return true;
   };
 
@@ -536,5 +536,5 @@ QueryData genAuthenticode(QueryContext& context) {
 
   return results;
 }
-}
-}
+} // namespace tables
+} // namespace osquery

osquery/tables/system/windows/certificates.cpp

@@ -228,7 +228,7 @@ std::string getUsernameFromSid(const std::string& sidString) {
     return "";
   }
 
-  return wstringToString(uname.data());
+  return wstringToString(uname.data(), uname.size());
 }
 
 bool isValidSid(const std::string& maybeSid) {
@@ -414,44 +414,44 @@ void addCertRow(PCCERT_CONTEXT certContext,
   r["username"] = username;
   r["store_id"] = storeId;
   r["sha1"] = fingerprint;
-  std::vector<WCHAR> certBuff;
-  certBuff.resize(256, 0);
-  std::fill(certBuff.begin(), certBuff.end(), 0);
+  std::vector<wchar_t> certBuff(256);
   CertGetNameString(certContext,
                     CERT_NAME_SIMPLE_DISPLAY_TYPE,
                     0,
                     nullptr,
                     certBuff.data(),
-                    static_cast<unsigned long>(certBuff.size()));
-  r["common_name"] = wstringToString(certBuff.data());
+                    certBuff.size());
+  r["common_name"] = wstringToString(certBuff.data(), certBuff.size());
 
   auto subjSize = CertNameToStr(certContext->dwCertEncodingType,
                                 &(certContext->pCertInfo->Subject),
                                 CERT_SIMPLE_NAME_STR,
                                 nullptr,
                                 0);
-  certBuff.resize(subjSize, 0);
+  certBuff.resize(subjSize);
   std::fill(certBuff.begin(), certBuff.end(), 0);
   subjSize = CertNameToStr(certContext->dwCertEncodingType,
                            &(certContext->pCertInfo->Subject),
                            CERT_SIMPLE_NAME_STR,
                            certBuff.data(),
                            subjSize);
-  r["subject"] = subjSize == 0 ? "" : wstringToString(certBuff.data());
+  r["subject"] =
+      subjSize == 0 ? "" : wstringToString(certBuff.data(), certBuff.size());
 
   auto issuerSize = CertNameToStr(certContext->dwCertEncodingType,
                                   &(certContext->pCertInfo->Issuer),
                                   CERT_SIMPLE_NAME_STR,
                                   nullptr,
                                   0);
-  certBuff.resize(issuerSize, 0);
+  certBuff.resize(issuerSize);
   std::fill(certBuff.begin(), certBuff.end(), 0);
   issuerSize = CertNameToStr(certContext->dwCertEncodingType,
                              &(certContext->pCertInfo->Issuer),
                              CERT_SIMPLE_NAME_STR,
                              certBuff.data(),
                              issuerSize);
-  r["issuer"] = issuerSize == 0 ? "" : wstringToString(certBuff.data());
+  r["issuer"] =
+      issuerSize == 0 ? "" : wstringToString(certBuff.data(), certBuff.size());
 
   // TODO(#5654) 1: Find the right API calls to get whether a cert is for a CA
   r["ca"] = INTEGER(-1);
@@ -513,7 +513,7 @@ void addCertRow(PCCERT_CONTEXT certContext,
                           nullptr,
                           &decodedBuffSize);
 
-      certBuff.resize(decodedBuffSize, 0);
+      certBuff.resize(decodedBuffSize);
       std::fill(certBuff.begin(), certBuff.end(), 0);
       auto decodeRet = CryptDecodeObjectEx(CERT_ENCODING,
                                            X509_AUTHORITY_KEY_ID2,
@@ -543,21 +543,21 @@ void addCertRow(PCCERT_CONTEXT certContext,
 
 Status expandEnvironmentVariables(const std::string& src, std::string& dest) {
   auto srcWstring = stringToWstring(src);
-  auto srcW = srcWstring.c_str();
-  auto expandedSize = ExpandEnvironmentStringsW(srcW, nullptr, 0);
+  auto expandedSize = ExpandEnvironmentStringsW(srcWstring.c_str(), nullptr, 0);
   if (expandedSize == 0) {
     return Status::failure("Unable to get expanded size");
   }
 
   std::vector<wchar_t> buf(expandedSize);
-  auto ret = ExpandEnvironmentStringsW(srcW, buf.data(), expandedSize);
+  auto ret =
+      ExpandEnvironmentStringsW(srcWstring.c_str(), buf.data(), expandedSize);
   if (ret == 0) {
     return Status::failure("Environment variable expansion failed");
   } else if (ret != expandedSize) {
     return Status::failure("Partial data written");
   }
 
-  dest = wstringToString(buf.data());
+  dest = wstringToString(buf.data(), buf.size());
   return Status::success();
 }
 

osquery/tables/system/windows/drivers.cpp

@@ -17,13 +17,13 @@
 #include <cfgmgr32.h>
 // clang-format on
 
+#include <osquery/core/windows/wmi.h>
 #include <osquery/logger/logger.h>
 #include <osquery/sql/sql.h>
+#include <osquery/tables/system/windows/registry.h>
 #include <osquery/utils/conversions/tryto.h>
-#include <osquery/core/windows/wmi.h>
 #include <osquery/utils/conversions/windows/strings.h>
 #include <osquery/utils/conversions/windows/windows_time.h>
-#include <osquery/tables/system/windows/registry.h>
 
 #include <boost/algorithm/string/case_conv.hpp>
 #include <boost/filesystem.hpp>
@@ -75,7 +75,8 @@ static inline std::string kNormalizeImage(std::string& path) {
   if (path.find("system32") != std::string::npos) {
     path = boost::regex_replace(path, boost::regex("^.*?system32"), "");
   }
-  boost::filesystem::path normalized_path(wstringToString(sys_root));
+  boost::filesystem::path normalized_path(
+      wstringToString(sys_root.data(), sys_root.size()));
   normalized_path /= path;
   return normalized_path.string();
 }
@@ -161,7 +162,7 @@ Status getDeviceProperty(const device_infoset_t& infoset,
   } else if (dev_prop_type == DEVPROP_TYPE_INT32) {
     result = std::to_string(*(PINT32)drv_buff.get());
   } else if (dev_prop_type == DEVPROP_TYPE_STRING) {
-    result = wstringToString((PWCHAR)drv_buff.get());
+    result = wstringToString((PWCHAR)drv_buff.get(), buff_size / sizeof(WCHAR));
   } else if (dev_prop_type == DEVPROP_TYPE_FILETIME) {
     result =
         std::to_string(osquery::filetimeToUnixtime(*(PFILETIME)drv_buff.get()));
@@ -208,7 +209,9 @@ Status getDriverImagePath(const std::string& svc_name, std::string& result) {
     return Status(ret, "Failed to query registry value(length)");
   }
 
-  auto buff = std::make_unique<WCHAR[]>(buff_size / sizeof(WCHAR));
+  const auto buff_chars = buff_size / sizeof(WCHAR);
+
+  auto buff = std::make_unique<WCHAR[]>(buff_chars);
   ret = RegGetValueW(hkey,
                      nullptr,
                      image_path_value,
@@ -220,7 +223,7 @@ Status getDriverImagePath(const std::string& svc_name, std::string& result) {
     return Status(ret, "Failed to query registry value");
   }
 
-  auto path = wstringToString(buff.get());
+  auto path = wstringToString(buff.get(), buff_chars);
   result = kNormalizeImage(path);
   return Status::success();
 }
@@ -369,7 +372,7 @@ QueryData genDrivers(QueryContext& context) {
                 << r["device_name"] << " with " << GetLastError();
         r["inf"] = wstringToString(inf_name);
       } else {
-        r["inf"] = wstringToString(inf.data());
+        r["inf"] = wstringToString(inf.data(), inf.size());
       }
     }
 

osquery/tables/system/windows/ie_extensions.cpp

@@ -18,10 +18,10 @@
 #include <osquery/logger/logger.h>
 #include <osquery/sql/sql.h>
 
-#include <osquery/utils/conversions/tryto.h>
 #include "osquery/core/windows/wmi.h"
 #include "osquery/filesystem/fileops.h"
 #include "osquery/tables/system/windows/registry.h"
+#include <osquery/utils/conversions/tryto.h>
 
 namespace osquery {
 namespace tables {

osquery/tables/system/windows/intel_me.cpp

@@ -28,8 +28,8 @@
 #include <osquery/core/tables.h>
 #include <osquery/logger/logger.h>
 
-#include <osquery/utils/conversions/tryto.h>
 #include <osquery/tables/system/intel_me.hpp>
+#include <osquery/utils/conversions/tryto.h>
 
 // The AMT documentation can be found at the following address:
 // https://software.intel.com/sites/manageability/AMT_Implementation_and_Reference_Guide/default.htm
@@ -333,15 +333,18 @@ osquery::Status getDeviceInterfacePath(
         std::to_string(err));
   }
 
-  std::wstring path;
-  path.assign(device_details->DevicePath, buffer.size() - sizeof(DWORD));
+  const auto device_path_size =
+      wcsnlen(device_details->DevicePath,
+              (buffer.size() - sizeof(DWORD)) / sizeof(WCHAR));
 
-  if (std::wcslen(path.c_str()) == 0U) {
+  if (device_path_size == 0U) {
     return osquery::Status::failure(
         "Invalid path returned for the given device interface; the string is "
         "empty");
   }
 
+  std::wstring path(device_details->DevicePath, device_path_size);
+
   dev_interface_path = wstringToString(path);
   return osquery::Status::success();
 }

osquery/tables/system/windows/kernel_info.cpp

@@ -39,7 +39,7 @@ void GetSystemDriveGUID(Row& r) {
   auto sysRoot = getSystemRoot().root_name().wstring() + L"\\";
   if (GetVolumeNameForVolumeMountPoint(
           sysRoot.c_str(), static_cast<LPWSTR>(buf), 50)) {
-    r["device"] = SQL_TEXT(wstringToString(buf));
+    r["device"] = SQL_TEXT(wstringToString(buf, ARRAYSIZE(buf)));
   }
 }