From 64ab0849719f07472e5314dd926eee19d52abbc4 Mon Sep 17 00:00:00 2001 From: Ateeq Sharfuddin Date: Sun, 19 Jan 2020 17:36:49 -0500 Subject: [PATCH 01/16] fix: on Windows use UTF-8 strings instead of system default locale strings --- osquery/config/tests/test_utils.cpp | 7 +- osquery/core/init.cpp | 2 +- osquery/core/system.cpp | 13 +- osquery/core/tests/process_tests.cpp | 16 +- osquery/core/tests/windows/wmi_tests.cpp | 6 +- osquery/core/watcher.cpp | 4 +- osquery/devtools/printer.cpp | 4 +- osquery/devtools/shell.cpp | 4 +- .../events/windows/ntfs_event_publisher.cpp | 87 +++++----- osquery/events/windows/usn_journal_reader.cpp | 26 +-- osquery/filesystem/tests/filesystem.cpp | 8 +- osquery/filesystem/windows/fileops.cpp | 163 ++++++++++-------- osquery/main/windows/main.cpp | 8 +- osquery/process/windows/process.cpp | 31 ++-- osquery/process/windows/process_ops.cpp | 12 +- osquery/remote/enroll/enroll.cpp | 7 +- osquery/tables/networking/etc_protocols.cpp | 6 +- osquery/tables/networking/windows/routes.cpp | 8 +- .../tables/system/windows/authenticode.cpp | 28 +-- .../system/windows/chocolatey_packages.cpp | 2 +- .../tables/system/windows/logged_in_users.cpp | 18 +- .../system/windows/ntfs_acl_permissions.cpp | 50 +++--- osquery/tables/system/windows/pipes.cpp | 15 +- osquery/tables/system/windows/processes.cpp | 48 +++--- osquery/tables/system/windows/registry.cpp | 115 ++++++------ .../tables/system/windows/scheduled_tasks.cpp | 4 +- .../tables/system/windows/startup_items.cpp | 4 +- .../tables/system/windows/windows_crashes.cpp | 6 +- osquery/utils/aws/tests/aws_util_tests.cpp | 20 ++- osquery/utils/system/env.h | 6 +- osquery/utils/system/errno.h | 2 +- osquery/utils/system/windows/env.cpp | 18 +- osquery/utils/system/windows/errno.cpp | 10 +- .../integration/tables/chrome_extensions.cpp | 1 + 34 files changed, 401 insertions(+), 358 deletions(-) diff --git a/osquery/config/tests/test_utils.cpp b/osquery/config/tests/test_utils.cpp index 207a23efcbf..95403f1bd25 100644 --- a/osquery/config/tests/test_utils.cpp +++ b/osquery/config/tests/test_utils.cpp @@ -10,6 +10,7 @@ #include +#include #include #include @@ -23,10 +24,12 @@ namespace { namespace fs = boost::filesystem; fs::path getConfDirPathImpl() { - char const* kEnvVarName = "TEST_CONF_FILES_DIR"; + std::wstring kEnvVarName = L"TEST_CONF_FILES_DIR"; auto const value_opt = osquery::getEnvVar(kEnvVarName); EXPECT_TRUE(static_cast(value_opt)) - << "Env var " << boost::io::quoted(kEnvVarName) << " was not found, " + << "Env var " + << boost::io::quoted(osquery::wstringToString(kEnvVarName.c_str())) + << " was not found, " << " looks like cxx_test argument 'env' is not set up."; return fs::path(value_opt.get()); } diff --git a/osquery/core/init.cpp b/osquery/core/init.cpp index e7f9c777e99..12dc28545dd 100644 --- a/osquery/core/init.cpp +++ b/osquery/core/init.cpp @@ -97,7 +97,7 @@ CLI_FLAG(bool, namespace { extern "C" { static inline bool hasWorkerVariable() { - return ::osquery::getEnvVar("OSQUERY_WORKER").is_initialized(); + return ::osquery::getEnvVar(L"OSQUERY_WORKER").is_initialized(); } volatile std::sig_atomic_t kHandledSignal{0}; diff --git a/osquery/core/system.cpp b/osquery/core/system.cpp index 8409e21f8cd..60984e71c5d 100644 --- a/osquery/core/system.cpp +++ b/osquery/core/system.cpp @@ -157,12 +157,17 @@ std::string getFqdn() { #endif return fqdn_string; } else { - unsigned long size = 256; - std::vector fqdn(size, 0x0); + std::string result; #ifdef WIN32 - GetComputerNameEx(ComputerNameDnsFullyQualified, fqdn.data(), &size); + DWORD size = 0; + if (0 == GetComputerNameExW(ComputerNameDnsFullyQualified, NULL, &size)) { + std::vector fqdn(size, 0x0); + GetComputerNameExW(ComputerNameDnsFullyQualified, fqdn.data(), &size); + result = wstringToString(fqdn.data()); + } + #endif - return fqdn.data(); + return result; } } diff --git a/osquery/core/tests/process_tests.cpp b/osquery/core/tests/process_tests.cpp index 5f0d3a3446a..4bee2f7f649 100644 --- a/osquery/core/tests/process_tests.cpp +++ b/osquery/core/tests/process_tests.cpp @@ -153,20 +153,20 @@ TEST_F(ProcessTests, test_getpid) { } TEST_F(ProcessTests, test_envVar) { - auto val = getEnvVar("GTEST_OSQUERY"); + auto val = getEnvVar(L"GTEST_OSQUERY"); EXPECT_FALSE(val); EXPECT_FALSE(val.is_initialized()); - EXPECT_TRUE(setEnvVar("GTEST_OSQUERY", "true")); + EXPECT_TRUE(setEnvVar(L"GTEST_OSQUERY", L"true")); - val = getEnvVar("GTEST_OSQUERY"); + val = getEnvVar(L"GTEST_OSQUERY"); EXPECT_FALSE(!val); EXPECT_TRUE(val.is_initialized()); - EXPECT_EQ(*val, "true"); + EXPECT_EQ(*val, L"true"); - EXPECT_TRUE(unsetEnvVar("GTEST_OSQUERY")); + EXPECT_TRUE(unsetEnvVar(L"GTEST_OSQUERY")); - val = getEnvVar("GTEST_OSQUERY"); + val = getEnvVar(L"GTEST_OSQUERY"); EXPECT_FALSE(val); EXPECT_FALSE(val.is_initialized()); } @@ -302,9 +302,9 @@ int main(int argc, char* argv[]) { osquery::kExpectedExtensionArgs[0] = argv[0]; osquery::kExpectedWorkerArgs[0] = argv[0]; - if (auto val = osquery::getEnvVar("OSQUERY_WORKER")) { + if (auto val = osquery::getEnvVar(L"OSQUERY_WORKER")) { return workerMain(argc, argv); - } else if ((val = osquery::getEnvVar("OSQUERY_EXTENSION"))) { + } else if ((val = osquery::getEnvVar(L"OSQUERY_EXTENSION"))) { return extensionMain(argc, argv); } diff --git a/osquery/core/tests/windows/wmi_tests.cpp b/osquery/core/tests/windows/wmi_tests.cpp index 1e0640b19da..1abd7eb3c86 100644 --- a/osquery/core/tests/windows/wmi_tests.cpp +++ b/osquery/core/tests/windows/wmi_tests.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include "osquery/core/windows/wmi.h" @@ -31,11 +32,12 @@ class WmiTests : public testing::Test { }; TEST_F(WmiTests, test_methodcall_inparams) { - auto windir = getEnvVar("WINDIR"); + auto windir = getEnvVar(L"WINDIR"); EXPECT_TRUE(windir); std::stringstream ss; - ss << "SELECT * FROM Win32_Directory WHERE Name = \"" << *windir << "\""; + ss << "SELECT * FROM Win32_Directory WHERE Name = \"" + << wstringToString(windir.get().c_str()) << "\""; auto query = ss.str(); diff --git a/osquery/core/watcher.cpp b/osquery/core/watcher.cpp index 49d439aabb1..7abd0f8dd66 100644 --- a/osquery/core/watcher.cpp +++ b/osquery/core/watcher.cpp @@ -182,7 +182,7 @@ bool Watcher::hasManagedExtensions() const { // Setting this counter to 0 will prevent the worker from waiting for missing // dependent config plugins. Otherwise, its existence, will cause a worker to // wait for missing plugins to broadcast from managed extensions. - return getEnvVar("OSQUERY_EXTENSIONS").is_initialized(); + return getEnvVar(L"OSQUERY_EXTENSIONS").is_initialized(); } bool WatcherRunner::ok() const { @@ -551,7 +551,7 @@ void WatcherRunner::createWorker() { // Set an environment signaling to potential plugin-dependent workers to wait // for extensions to broadcast. if (watcher.hasManagedExtensions()) { - setEnvVar("OSQUERY_EXTENSIONS", "true"); + setEnvVar(L"OSQUERY_EXTENSIONS", L"true"); } // Get the complete path of the osquery process binary. diff --git a/osquery/devtools/printer.cpp b/osquery/devtools/printer.cpp index a4274027cfc..6dbd8f99430 100644 --- a/osquery/devtools/printer.cpp +++ b/osquery/devtools/printer.cpp @@ -29,7 +29,7 @@ std::string generateToken(const std::map& lengths, std::string out = "+"; for (const auto& col : columns) { size_t size = tryTakeCopy(lengths, col).takeOr(col.size()) + 2; - if (getEnvVar("ENHANCE").is_initialized()) { + if (getEnvVar(L"ENHANCE").is_initialized()) { std::string e = "\xF0\x9F\x90\x8C"; e[2] += kOffset[1]; e[3] += kOffset[0]; @@ -58,7 +58,7 @@ std::string generateToken(const std::map& lengths, std::string generateHeader(const std::map& lengths, const std::vector& columns) { - if (getEnvVar("ENHANCE").is_initialized()) { + if (getEnvVar(L"ENHANCE").is_initialized()) { kToken = "\xF0\x9F\x91\x8D"; } std::string out = kToken; diff --git a/osquery/devtools/shell.cpp b/osquery/devtools/shell.cpp index a6d91057786..a00859c7196 100644 --- a/osquery/devtools/shell.cpp +++ b/osquery/devtools/shell.cpp @@ -1641,9 +1641,9 @@ static void main_init(struct callback_data* data) { sqlite3_config(SQLITE_CONFIG_URI, 1); sqlite3_config(SQLITE_CONFIG_LOG, shellLog, data); - auto term = osquery::getEnvVar("TERM"); + auto term = osquery::getEnvVar(L"TERM"); if (term.is_initialized() && - (*term).find("xterm-256color") != std::string::npos) { + (*term).find(L"xterm-256color") != std::wstring::npos) { sqlite3_snprintf( sizeof(mainPrompt), mainPrompt, "\033[38;5;147mosquery> \033[0m"); sqlite3_snprintf(sizeof(continuePrompt), diff --git a/osquery/events/windows/ntfs_event_publisher.cpp b/osquery/events/windows/ntfs_event_publisher.cpp index f8a359f08cc..b2b9115833a 100644 --- a/osquery/events/windows/ntfs_event_publisher.cpp +++ b/osquery/events/windows/ntfs_event_publisher.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include "osquery/events/windows/ntfs_event_publisher.h" @@ -214,19 +215,19 @@ Status NTFSEventPublisher::getPathFromReferenceNumber( message << "Failed to open the file in volume " << drive_letter << ":\\. Error: "; - std::string description; + std::wstring description; if (!getWindowsErrorDescription(description, ::GetLastError())) { - description = "Unknown error"; + description = L"Unknown error"; } - message << description; + message << wstringToString(description.c_str()); return Status::failure(message.str()); } - auto required_bytes = static_cast(::GetFinalPathNameByHandle( + auto required_characters = static_cast(::GetFinalPathNameByHandleW( handle, nullptr, 0, FILE_NAME_NORMALIZED | VOLUME_NAME_DOS)); - if (required_bytes == 0U) { + if (required_characters == 0U) { auto error_code = ::GetLastError(); ::CloseHandle(handle); @@ -234,31 +235,31 @@ Status NTFSEventPublisher::getPathFromReferenceNumber( message << "Failed to determine the path size for the file in volume " << drive_letter << ":\\. Error: "; - std::string description; + std::wstring description; if (!getWindowsErrorDescription(description, error_code)) { - description = "Unknown error"; + description = L"Unknown error"; } - message << description; + message << wstringToString(description.c_str()); return Status::failure(message.str()); } // We are going to add an additional byte, as we may or may not have the null // terminator already included depending on the operating system version - std::string buffer; - required_bytes += 1U; + std::wstring buffer; + required_characters += 1U; - buffer.resize(required_bytes); - if (buffer.size() != required_bytes) { + buffer.resize(required_characters); + if (buffer.size() != required_characters) { ::CloseHandle(handle); throw std::bad_alloc(); } auto bytes_returned = static_cast( - ::GetFinalPathNameByHandle(handle, - &buffer[0], - static_cast(buffer.size()), - FILE_NAME_NORMALIZED | VOLUME_NAME_DOS)); + ::GetFinalPathNameByHandleW(handle, + &buffer[0], + static_cast(buffer.size()), + FILE_NAME_NORMALIZED | VOLUME_NAME_DOS)); auto error_code = ::GetLastError(); ::CloseHandle(handle); @@ -268,17 +269,17 @@ Status NTFSEventPublisher::getPathFromReferenceNumber( message << "Failed to acquire the path for the file in volume " << drive_letter << ":\\. Error: "; - std::string description; + std::wstring description; if (!getWindowsErrorDescription(description, error_code)) { - description = "Unknown error"; + description = L"Unknown error"; } - message << description; + message << wstringToString(description.c_str()); return Status::failure(message.str()); } // Paths follow this form: \\?\C:\\path\\to\\folder; skip the prefix - path = buffer.c_str() + 4; + path = wstringToString(buffer.c_str() + 4); buffer.clear(); return Status::success(); @@ -328,25 +329,25 @@ Status NTFSEventPublisher::getVolumeData(VolumeData& volume, VolumeData volume_data = {}; volume_data.volume_handle = - ::CreateFile(volume_path.c_str(), - GENERIC_READ, - FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, - nullptr, - OPEN_EXISTING, - FILE_FLAG_BACKUP_SEMANTICS, - nullptr); + ::CreateFileW(stringToWstring(volume_path).c_str(), + GENERIC_READ, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + nullptr, + OPEN_EXISTING, + FILE_FLAG_BACKUP_SEMANTICS, + nullptr); if (volume_data.volume_handle == INVALID_HANDLE_VALUE) { std::stringstream message; message << "Failed to open the following drive: " << volume_path << " due to the following error: "; - std::string description; + std::wstring description; if (!getWindowsErrorDescription(description, ::GetLastError())) { - description = "Unknown error"; + description = L"Unknown error"; } - message << description; + message << wstringToString(description.c_str()); return Status::failure(message.str()); } @@ -356,13 +357,13 @@ Status NTFSEventPublisher::getVolumeData(VolumeData& volume, root_folder_path.append(":\\"); volume_data.root_folder_handle = - ::CreateFile(root_folder_path.c_str(), - GENERIC_READ, - FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, - nullptr, - OPEN_EXISTING, - FILE_FLAG_BACKUP_SEMANTICS, - nullptr); + ::CreateFileW(stringToWstring(root_folder_path).c_str(), + GENERIC_READ, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + nullptr, + OPEN_EXISTING, + FILE_FLAG_BACKUP_SEMANTICS, + nullptr); if (volume_data.root_folder_handle == INVALID_HANDLE_VALUE) { auto error_code = ::GetLastError(); @@ -372,12 +373,12 @@ Status NTFSEventPublisher::getVolumeData(VolumeData& volume, message << "Failed to get the root folder handle for volume '" << drive_letter << "'. Error: "; - std::string description; + std::wstring description; if (!getWindowsErrorDescription(description, error_code)) { - description = "Unknown error"; + description = L"Unknown error"; } - message << description; + message << wstringToString(description.c_str()); return Status::failure(message.str()); } @@ -402,12 +403,12 @@ Status NTFSEventPublisher::getVolumeData(VolumeData& volume, message << "Failed to get the root reference number for volume '" << drive_letter << "'. Error: "; - std::string description; + std::wstring description; if (!getWindowsErrorDescription(description, error_code)) { - description = "Unknown error"; + description = L"Unknown error"; } - message << description; + message << wstringToString(description.c_str()); return Status::failure(message.str()); } diff --git a/osquery/events/windows/usn_journal_reader.cpp b/osquery/events/windows/usn_journal_reader.cpp index 8ae654e576f..d3f89bca545 100644 --- a/osquery/events/windows/usn_journal_reader.cpp +++ b/osquery/events/windows/usn_journal_reader.cpp @@ -236,13 +236,13 @@ Status USNJournalReader::initialize() { std::string("\\\\.\\") + d_->journal_reader_context->drive_letter + ":"; d_->volume_handle = - ::CreateFile(d_->volume_path.c_str(), - FILE_GENERIC_READ, - FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, - nullptr, - OPEN_EXISTING, - 0, - nullptr); + ::CreateFileW(stringToWstring(d_->volume_path).c_str(), + FILE_GENERIC_READ, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + nullptr, + OPEN_EXISTING, + 0, + nullptr); if (d_->volume_handle == INVALID_HANDLE_VALUE) { std::stringstream error_message; @@ -275,11 +275,11 @@ Status USNJournalReader::initialize() { "number for the following volume: " << d_->volume_path << ". Error message: "; - std::string description; + std::wstring description; if (!getWindowsErrorDescription(description, error_code)) { - description = "Unknown error"; + description = L"Unknown error"; } - error_message << description; + error_message << wstringToString(description.c_str()); return Status::failure(error_message.str()); } @@ -328,11 +328,11 @@ Status USNJournalReader::acquireRecords() { error_message << "Failed to read the journal of the following volume: " << d_->volume_path << ". Error message: "; - std::string description; + std::wstring description; if (!getWindowsErrorDescription(description, ::GetLastError())) { - description = "Unknown error"; + description = L"Unknown error"; } - error_message << description; + error_message << wstringToString(description.c_str()); return Status::failure(error_message.str()); } diff --git a/osquery/filesystem/tests/filesystem.cpp b/osquery/filesystem/tests/filesystem.cpp index 88919d9574d..8604b034198 100644 --- a/osquery/filesystem/tests/filesystem.cpp +++ b/osquery/filesystem/tests/filesystem.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -55,8 +56,11 @@ class FilesystemTests : public testing::Test { tmp_path_ = fs::temp_directory_path().string(); line_ending_ = "\r\n"; - auto raw_drive = getEnvVar("SystemDrive"); - system_root_ = (raw_drive.is_initialized() ? *raw_drive : "") + "\\"; + auto raw_drive = getEnvVar(L"SystemDrive"); + system_root_ = + (raw_drive.is_initialized() ? wstringToString(raw_drive.get().c_str()) + : "") + + "\\"; } else { etc_hosts_path_ = "/etc/hosts"; etc_path_ = "/etc"; diff --git a/osquery/filesystem/windows/fileops.cpp b/osquery/filesystem/windows/fileops.cpp index 19e78071924..db6b80bdb37 100644 --- a/osquery/filesystem/windows/fileops.cpp +++ b/osquery/filesystem/windows/fileops.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -53,7 +54,8 @@ using AclObject = std::unique_ptr; class WindowsFindFiles { public: explicit WindowsFindFiles(const fs::path& path) : path_(path) { - handle_ = ::FindFirstFileA(path_.make_preferred().string().c_str(), &fd_); + handle_ = ::FindFirstFileW( + stringToWstring(path_.make_preferred().string()).c_str(), &fd_); } ~WindowsFindFiles() { @@ -68,7 +70,7 @@ class WindowsFindFiles { if (handle_ != INVALID_HANDLE_VALUE) { do { - std::string component(fd_.cFileName); + std::string component = wstringToString(fd_.cFileName); if (component != "." && component != "..") { if (path_.has_parent_path()) { results.push_back(path_.parent_path() / component); @@ -78,7 +80,7 @@ class WindowsFindFiles { } ::RtlZeroMemory(&fd_, sizeof(fd_)); - } while (::FindNextFileA(handle_, &fd_)); + } while (::FindNextFileW(handle_, &fd_)); } return results; @@ -99,30 +101,33 @@ class WindowsFindFiles { private: HANDLE handle_{INVALID_HANDLE_VALUE}; - WIN32_FIND_DATAA fd_{0}; + WIN32_FIND_DATAW fd_{0}; fs::path path_; }; Status windowsShortPathToLongPath(const std::string& shortPath, std::string& rLongPath) { - TCHAR longPath[MAX_PATH]; - auto ret = GetLongPathName(shortPath.c_str(), longPath, MAX_PATH); + WCHAR longPath[MAX_PATH]; + auto ret = + GetLongPathNameW(stringToWstring(shortPath).c_str(), longPath, MAX_PATH); if (ret == 0) { return Status(GetLastError(), "Failed to convert short path to long path"); } - rLongPath = std::string(longPath); + rLongPath = wstringToString(longPath); return Status::success(); } Status windowsGetFileVersion(const std::string& path, std::string& rVersion) { DWORD handle = 0; - auto verSize = GetFileVersionInfoSize(path.c_str(), &handle); + std::wstring wpath = stringToWstring(path).c_str(); + auto verSize = GetFileVersionInfoSizeW(wpath.c_str(), &handle); auto verInfo = std::make_unique(verSize); if (verInfo == nullptr) { return Status(1, "Failed to malloc for version info"); } - auto err = GetFileVersionInfo(path.c_str(), handle, verSize, verInfo.get()); + auto err = GetFileVersionInfoW( + stringToWstring(path).c_str(), handle, verSize, verInfo.get()); if (err == 0) { return Status(GetLastError(), "Failed to get file version info"); } @@ -162,7 +167,7 @@ static bool hasGlobBraces(const std::string& glob) { } AsyncEvent::AsyncEvent() { - overlapped_.hEvent = ::CreateEventA(nullptr, FALSE, FALSE, nullptr); + overlapped_.hEvent = ::CreateEventW(nullptr, FALSE, FALSE, nullptr); } AsyncEvent::~AsyncEvent() { @@ -593,7 +598,7 @@ PlatformFile::PlatformFile(const fs::path& path, int mode, int perms) // TODO(#2001): set up a security descriptor based off the perms } - handle_ = ::CreateFileA(fname_.string().c_str(), + handle_ = ::CreateFileW(stringToWstring(fname_.string()).c_str(), access_mask, FILE_SHARE_READ, security_attrs.get(), @@ -857,19 +862,19 @@ Status PlatformFile::hasSafePermissions() const { SecurityDescriptor file_sd_wrapper(file_sd); // Get the access control list for the parent directory - std::vector path_buf(MAX_PATH + 1, '\0'); - if (GetFinalPathNameByHandleA( + std::vector path_buf(MAX_PATH + 1, '\0'); + if (GetFinalPathNameByHandleW( handle_, path_buf.data(), MAX_PATH, FILE_NAME_NORMALIZED) == 0) { - return Status(-1, "GetFinalPathNameByHandleA failed"); + return Status(-1, "GetFinalPathNameByHandle failed"); } - if (!PathRemoveFileSpecA(path_buf.data())) { + if (!PathRemoveFileSpecW(path_buf.data())) { return Status(-1, "PathRemoveFileSpec"); } PACL dir_dacl = nullptr; PSECURITY_DESCRIPTOR dir_sd = nullptr; - if (GetNamedSecurityInfoA(path_buf.data(), + if (GetNamedSecurityInfoW(path_buf.data(), SE_FILE_OBJECT, DACL_SECURITY_INFORMATION, nullptr, @@ -1111,18 +1116,18 @@ bool platformSetSafeDbPerms(const std::string& path) { return false; } - EXPLICIT_ACCESS admins_ea; + EXPLICIT_ACCESSW admins_ea; admins_ea.grfAccessMode = SET_ACCESS; admins_ea.grfAccessPermissions = GENERIC_ALL; admins_ea.grfInheritance = SUB_CONTAINERS_AND_OBJECTS_INHERIT; - PTRUSTEE_A trust = static_cast(malloc(sizeof(TRUSTEE_A))); - BuildTrusteeWithSidA(trust, admins); + PTRUSTEE_W trust = static_cast(malloc(sizeof(TRUSTEE_W))); + BuildTrusteeWithSidW(trust, admins); admins_ea.Trustee = *trust; // Set the Administrators ACE PACL new_dacl = nullptr; - auto ret = SetEntriesInAcl(1, &admins_ea, nullptr, &new_dacl); + auto ret = SetEntriesInAclW(1, &admins_ea, nullptr, &new_dacl); if (ret != ERROR_SUCCESS) { VLOG(1) << "Failed to set DB permissions for Administrators"; LocalFree(new_dacl); @@ -1131,14 +1136,14 @@ bool platformSetSafeDbPerms(const std::string& path) { } // Set the SYSTEM ACE - EXPLICIT_ACCESS sys_ea; + EXPLICIT_ACCESSW sys_ea; sys_ea.grfAccessMode = SET_ACCESS; sys_ea.grfAccessPermissions = GENERIC_ALL; sys_ea.grfInheritance = SUB_CONTAINERS_AND_OBJECTS_INHERIT; - BuildTrusteeWithSidA(trust, system); + BuildTrusteeWithSidW(trust, system); sys_ea.Trustee = *trust; - ret = SetEntriesInAcl(1, &sys_ea, new_dacl, &new_dacl); + ret = SetEntriesInAclW(1, &sys_ea, new_dacl, &new_dacl); if (ret != ERROR_SUCCESS) { VLOG(1) << "Failed to set DB permissions for SYSTEM"; LocalFree(new_dacl); @@ -1147,14 +1152,14 @@ bool platformSetSafeDbPerms(const std::string& path) { } // Grant Everyone the ability to read the DACL - EXPLICIT_ACCESS world_ea; + EXPLICIT_ACCESSW world_ea; world_ea.grfAccessMode = SET_ACCESS; world_ea.grfAccessPermissions = READ_CONTROL; world_ea.grfInheritance = SUB_CONTAINERS_AND_OBJECTS_INHERIT; - BuildTrusteeWithSidA(trust, world); + BuildTrusteeWithSidW(trust, world); world_ea.Trustee = *trust; - ret = SetEntriesInAcl(1, &world_ea, new_dacl, &new_dacl); + ret = SetEntriesInAclW(1, &world_ea, new_dacl, &new_dacl); if (ret != ERROR_SUCCESS) { VLOG(1) << "Failed to set DB permissions for SYSTEM"; LocalFree(new_dacl); @@ -1162,9 +1167,10 @@ bool platformSetSafeDbPerms(const std::string& path) { return false; } + std::wstring wide_path = stringToWstring(path.c_str()); // Apply 'safe' DACL and avoid returning to attempt applying the DACL - ret = SetNamedSecurityInfoA( - const_cast(path.c_str()), + ret = SetNamedSecurityInfoW( + const_cast(wide_path.c_str()), SE_FILE_OBJECT, OWNER_SECURITY_INFORMATION | GROUP_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION | PROTECTED_DACL_SECURITY_INFORMATION, @@ -1185,7 +1191,7 @@ bool platformChmod(const std::string& path, mode_t perms) { PSID owner = nullptr; PSECURITY_DESCRIPTOR sd = nullptr; - auto ret = GetNamedSecurityInfoA(path.c_str(), + auto ret = GetNamedSecurityInfoW(stringToWstring(path).c_str(), SE_FILE_OBJECT, OWNER_SECURITY_INFORMATION | GROUP_SECURITY_INFORMATION | @@ -1276,8 +1282,9 @@ bool platformChmod(const std::string& path, mode_t perms) { return false; } + std::wstring wide_path = stringToWstring(path); // Lastly, apply the permissions to the object - if (SetNamedSecurityInfoA(const_cast(path.c_str()), + if (SetNamedSecurityInfoW(const_cast(wide_path.c_str()), SE_FILE_OBJECT, DACL_SECURITY_INFORMATION, nullptr, @@ -1299,9 +1306,10 @@ std::vector platformGlob(const std::string& find_path) { */ if (find_path.size() >= 2 && find_path[0] == '~' && (find_path[1] == '/' || find_path[1] == '\\')) { - auto homedir = getEnvVar("USERPROFILE"); + auto homedir = getEnvVar(L"USERPROFILE"); if (homedir.is_initialized()) { - full_path = fs::path(*homedir) / find_path.substr(2); + std::wstring whomedir = *homedir; + full_path = fs::path(wstringToString(whomedir.c_str())) / find_path.substr(2); } } @@ -1414,13 +1422,13 @@ std::vector platformGlob(const std::string& find_path) { } boost::optional getHomeDirectory() { - std::vector profile(MAX_PATH); - auto value = getEnvVar("USERPROFILE"); + std::vector profile(MAX_PATH); + auto value = getEnvVar(L"USERPROFILE"); if (value.is_initialized()) { - return value; - } else if (SUCCEEDED(::SHGetFolderPathA( + return wstringToString(value.get().c_str()); + } else if (SUCCEEDED(::SHGetFolderPathW( nullptr, CSIDL_PROFILE, nullptr, 0, profile.data()))) { - return std::string(profile.data()); + return wstringToString(profile.data()); } else { return boost::none; } @@ -1439,11 +1447,11 @@ int platformAccess(const std::string& path, mode_t mode) { static std::string normalizeDirPath(const fs::path& path) { std::regex pattern(".*[*?\"|<>].*"); - std::vector full_path(MAX_PATH + 1); - std::vector final_path(MAX_PATH + 1); + std::vector full_path(MAX_PATH + 1); + std::vector final_path(MAX_PATH + 1); - full_path.assign(MAX_PATH + 1, '\0'); - final_path.assign(MAX_PATH + 1, '\0'); + full_path.assign(MAX_PATH + 1, L'\0'); + final_path.assign(MAX_PATH + 1, L'\0'); // Fail if illegal characters are detected in the path if (std::regex_match(path.string(), pattern)) { @@ -1451,14 +1459,16 @@ static std::string normalizeDirPath(const fs::path& path) { } // Obtain the full path of the fs::path object - DWORD nret = ::GetFullPathNameA( - path.string().c_str(), MAX_PATH, full_path.data(), nullptr); + DWORD nret = ::GetFullPathNameW(stringToWstring(path.string()).c_str(), + MAX_PATH, + full_path.data(), + nullptr); if (nret == 0) { return std::string(); } HANDLE handle = INVALID_HANDLE_VALUE; - handle = ::CreateFileA(full_path.data(), + handle = ::CreateFileW(full_path.data(), GENERIC_READ, FILE_SHARE_READ, nullptr, @@ -1470,7 +1480,7 @@ static std::string normalizeDirPath(const fs::path& path) { } // Resolve any symbolic links (somewhat rare on Windows) - nret = ::GetFinalPathNameByHandleA( + nret = ::GetFinalPathNameByHandleW( handle, final_path.data(), MAX_PATH, FILE_NAME_NORMALIZED); ::CloseHandle(handle); @@ -1479,10 +1489,14 @@ static std::string normalizeDirPath(const fs::path& path) { } // NTFS is case insensitive, to normalize, make everything uppercase - ::CharUpperA(final_path.data()); + // TODO: the above comment is incorrect: + // 1. NTFS is actually case-sensitive. + // 2. OS might have case-sensitivity turned on, which will + // produce false-positives here. + ::CharUpperW(final_path.data()); boost::system::error_code ec; - std::string normalized_path(final_path.data(), nret); + std::string normalized_path = wstringToString(final_path.data()); if ((fs::is_directory(normalized_path, ec) && ec.value() == errc::success) && normalized_path[nret - 1] != '\\') { normalized_path += "\\"; @@ -1545,7 +1559,7 @@ boost::optional platformFopen(const std::string& filename, */ Status socketExists(const fs::path& path, bool remove_socket) { DWORD timeout = (remove_socket) ? 0 : 500; - if (::WaitNamedPipeA(path.string().c_str(), timeout) == 0) { + if (::WaitNamedPipeW(stringToWstring(path.string()).c_str(), timeout) == 0) { DWORD error = ::GetLastError(); if (error == ERROR_BAD_PATHNAME) { return Status(1, "Named pipe path is invalid"); @@ -1636,19 +1650,19 @@ std::string getFileAttribStr(unsigned long file_attributes) { } std::string lastErrorMessage(unsigned long error_code) { - LPTSTR msg_buffer = nullptr; + LPWSTR msg_buffer = nullptr; - FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | - FORMAT_MESSAGE_IGNORE_INSERTS, - NULL, - error_code, - MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), - (LPTSTR)&msg_buffer, - 0, - NULL); + FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | + FORMAT_MESSAGE_IGNORE_INSERTS, + NULL, + error_code, + MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), + (LPWSTR)&msg_buffer, + 0, + NULL); if (msg_buffer != NULL) { - auto error_message = std::string(msg_buffer); + auto error_message = wstringToString(msg_buffer); LocalFree(msg_buffer); msg_buffer = nullptr; @@ -1672,13 +1686,13 @@ Status platformStat(const fs::path& path, WINDOWS_STAT* wfile_stat) { } // Get the handle of the file object. - auto file_handle = CreateFile(path.string().c_str(), - GENERIC_READ, - FILE_SHARE_READ | FILE_SHARE_WRITE, - nullptr, - OPEN_EXISTING, - FLAGS_AND_ATTRIBUTES, - nullptr); + auto file_handle = CreateFileW(stringToWstring(path.string()).c_str(), + GENERIC_READ, + FILE_SHARE_READ | FILE_SHARE_WRITE, + nullptr, + OPEN_EXISTING, + FLAGS_AND_ATTRIBUTES, + nullptr); // Check GetLastError for CreateFile error code. if (file_handle == INVALID_HANDLE_VALUE) { @@ -1794,7 +1808,8 @@ Status platformStat(const fs::path& path, WINDOWS_STAT* wfile_stat) { : wfile_stat->size = li.QuadPart; const char* drive_letter = nullptr; - auto drive_letter_index = PathGetDriveNumber(path.string().c_str()); + auto drive_letter_index = + PathGetDriveNumberW(stringToWstring(path.string()).c_str()); if (drive_letter_index != -1 && kDriveLetters.count(drive_letter_index)) { drive_letter = kDriveLetters.at(drive_letter_index).c_str(); @@ -1804,11 +1819,11 @@ Status platformStat(const fs::path& path, WINDOWS_STAT* wfile_stat) { unsigned long free_clusters; unsigned long total_clusters; - if (GetDiskFreeSpace(drive_letter, - §_per_cluster, - &bytes_per_sect, - &free_clusters, - &total_clusters) != 0) { + if (GetDiskFreeSpaceA(drive_letter, + §_per_cluster, + &bytes_per_sect, + &free_clusters, + &total_clusters) != 0) { wfile_stat->block_size = bytes_per_sect; } else { wfile_stat->block_size = -1; @@ -1838,10 +1853,10 @@ Status platformStat(const fs::path& path, WINDOWS_STAT* wfile_stat) { } fs::path getSystemRoot() { - std::vector winDirectory(MAX_PATH + 1); + std::vector winDirectory(MAX_PATH + 1); ZeroMemory(winDirectory.data(), MAX_PATH + 1); - GetWindowsDirectory(winDirectory.data(), MAX_PATH); - return fs::path(std::string(winDirectory.data())); + GetWindowsDirectoryW(winDirectory.data(), MAX_PATH); + return fs::path(wstringToString(winDirectory.data())); } Status platformLstat(const std::string& path, struct stat& d_stat) { diff --git a/osquery/main/windows/main.cpp b/osquery/main/windows/main.cpp index b297a2d9ae1..36754aae85b 100644 --- a/osquery/main/windows/main.cpp +++ b/osquery/main/windows/main.cpp @@ -299,9 +299,9 @@ Status uninstallService() { } SC_HANDLE schService = - OpenService(schSCManager, - kServiceName.c_str(), - SERVICE_STOP | SERVICE_QUERY_STATUS | DELETE); + OpenServiceA(schSCManager, + kServiceName.c_str(), + SERVICE_STOP | SERVICE_QUERY_STATUS | DELETE); if (schService == nullptr) { CloseServiceHandle(schService); @@ -417,7 +417,7 @@ void WINAPI ServiceMain(DWORD argc, LPSTR* argv) { int main(int argc, char* argv[]) { SERVICE_TABLE_ENTRYA serviceTable[] = { {const_cast(osquery::kServiceName.c_str()), - static_cast(osquery::ServiceMain)}, + static_cast(osquery::ServiceMain)}, {nullptr, nullptr}}; if (!StartServiceCtrlDispatcherA(serviceTable)) { diff --git a/osquery/process/windows/process.cpp b/osquery/process/windows/process.cpp index aa5b83742e9..44966f4e668 100644 --- a/osquery/process/windows/process.cpp +++ b/osquery/process/windows/process.cpp @@ -15,6 +15,7 @@ #include #include +#include namespace fs = boost::filesystem; @@ -122,7 +123,7 @@ int PlatformProcess::getCurrentPid() { } std::shared_ptr PlatformProcess::getLauncherProcess() { - auto launcher_handle = getEnvVar("OSQUERY_LAUNCHER"); + auto launcher_handle = getEnvVar(L"OSQUERY_LAUNCHER"); if (!launcher_handle) { return std::make_shared(); } @@ -183,8 +184,8 @@ std::shared_ptr PlatformProcess::launchWorker( // OSQUERY_LAUNCHER. OSQUERY_LAUNCHER stores the string form of a HANDLE to // the current process. This is mostly used for detecting the death of the // launcher process in WatcherWatcherRunner::start - if (!setEnvVar("OSQUERY_WORKER", "1") || - !setEnvVar("OSQUERY_LAUNCHER", handle)) { + if (!setEnvVar(L"OSQUERY_WORKER", L"1") || + !setEnvVar(L"OSQUERY_LAUNCHER", stringToWstring(handle))) { ::CloseHandle(hLauncherProcess); return std::shared_ptr(); @@ -228,8 +229,8 @@ std::shared_ptr PlatformProcess::launchWorker( nullptr, &si, &pi); - unsetEnvVar("OSQUERY_WORKER"); - unsetEnvVar("OSQUERY_LAUNCHER"); + unsetEnvVar(L"OSQUERY_WORKER"); + unsetEnvVar(L"OSQUERY_LAUNCHER"); ::CloseHandle(hLauncherProcess); if (!status) { @@ -276,7 +277,7 @@ std::shared_ptr PlatformProcess::launchExtension( // 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 // this is never used elsewhere in the core. - if (!setEnvVar("OSQUERY_EXTENSION", "1")) { + if (!setEnvVar(L"OSQUERY_EXTENSION", L"1")) { return std::shared_ptr(); } @@ -297,7 +298,7 @@ std::shared_ptr PlatformProcess::launchExtension( nullptr, &si, &pi); - unsetEnvVar("OSQUERY_EXTENSION"); + unsetEnvVar(L"OSQUERY_EXTENSION"); if (!status) { return std::shared_ptr(); @@ -313,16 +314,16 @@ std::shared_ptr PlatformProcess::launchExtension( std::shared_ptr PlatformProcess::launchTestPythonScript( const std::string& args) { - STARTUPINFOA si = {0}; + STARTUPINFOW si = {0}; PROCESS_INFORMATION pi = {nullptr}; - auto argv = "python " + args; - std::vector mutable_argv(argv.begin(), argv.end()); - mutable_argv.push_back('\0'); + auto argv = L"python " + stringToWstring(args); + std::vector mutable_argv(argv.begin(), argv.end()); + mutable_argv.push_back(L'\0'); si.cb = sizeof(si); - auto pythonEnv = getEnvVar("OSQUERY_PYTHON_PATH"); - std::string pythonPath; + auto pythonEnv = getEnvVar(L"OSQUERY_PYTHON_PATH"); + std::wstring pythonPath; if (pythonEnv.is_initialized()) { pythonPath = *pythonEnv; } @@ -330,10 +331,10 @@ std::shared_ptr PlatformProcess::launchTestPythonScript( // Python is installed at this location if the provisioning script is used. // This path should work regardless of the existence of the SystemDrive // environment variable. - pythonPath += "\\python.exe"; + pythonPath += L"\\python.exe"; std::shared_ptr process; - if (::CreateProcessA(pythonPath.c_str(), + if (::CreateProcessW(pythonPath.c_str(), mutable_argv.data(), nullptr, nullptr, diff --git a/osquery/process/windows/process_ops.cpp b/osquery/process/windows/process_ops.cpp index d36abb9f7fe..c996fa97e1f 100644 --- a/osquery/process/windows/process_ops.cpp +++ b/osquery/process/windows/process_ops.cpp @@ -14,7 +14,7 @@ namespace osquery { std::string psidToString(PSID sid) { - LPTSTR sidOut = nullptr; + LPSTR sidOut = nullptr; auto ret = ConvertSidToStringSidA(sid, &sidOut); if (ret == 0) { VLOG(1) << "ConvertSidToString failed with " << GetLastError(); @@ -25,8 +25,8 @@ std::string psidToString(PSID sid) { uint32_t getUidFromSid(PSID sid) { auto const uid_default = static_cast(-1); - LPTSTR sidString; - if (ConvertSidToStringSid(sid, &sidString) == 0) { + LPSTR sidString; + if (ConvertSidToStringSidA(sid, &sidString) == 0) { VLOG(1) << "getUidFromSid failed ConvertSidToStringSid error " + std::to_string(GetLastError()); LocalFree(sidString); @@ -80,8 +80,8 @@ uint32_t getGidFromSid(PSID sid) { auto ret = NetUserGetInfo(nullptr, uname.data(), userInfoLevel, &userBuff); if (ret == NERR_UserNotFound) { - LPTSTR sidString; - ConvertSidToStringSid(sid, &sidString); + LPSTR sidString; + ConvertSidToStringSidA(sid, &sidString); auto toks = osquery::split(sidString, "-"); gid = tryTo(toks.at(toks.size() - 1), 10).takeOr(gid); LocalFree(sidString); @@ -200,7 +200,7 @@ bool isLauncherProcessDead(PlatformProcess& launcher) { } ModuleHandle platformModuleOpen(const std::string& path) { - return ::LoadLibraryA(path.c_str()); + return ::LoadLibraryW(stringToWstring(path).c_str()); } void* platformModuleGetSymbol(ModuleHandle module, const std::string& symbol) { diff --git a/osquery/remote/enroll/enroll.cpp b/osquery/remote/enroll/enroll.cpp index 021b6d76442..5ce60491f7a 100644 --- a/osquery/remote/enroll/enroll.cpp +++ b/osquery/remote/enroll/enroll.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include namespace osquery { @@ -97,9 +98,11 @@ const std::string getEnrollSecret() { readFile(FLAGS_enroll_secret_path, enrollment_secret); boost::trim(enrollment_secret); } else { - auto env_secret = getEnvVar(FLAGS_enroll_secret_env); + std::wstring wFLAGS_enroll_secret_env = + stringToWstring(FLAGS_enroll_secret_env); + auto env_secret = getEnvVar(wFLAGS_enroll_secret_env); if (env_secret.is_initialized()) { - enrollment_secret = *env_secret; + enrollment_secret = wstringToString(env_secret.get().c_str()); } } diff --git a/osquery/tables/networking/etc_protocols.cpp b/osquery/tables/networking/etc_protocols.cpp index 54cf1c0d486..8cd00297941 100644 --- a/osquery/tables/networking/etc_protocols.cpp +++ b/osquery/tables/networking/etc_protocols.cpp @@ -55,10 +55,10 @@ QueryData parseEtcProtocolsContent(const std::string& content) { } Row r; - r["name"] = TEXT(protocol_fields[0]); + r["name"] = SQL_TEXT(protocol_fields[0]); r["number"] = INTEGER(protocol_fields[1]); if (protocol_fields.size() > 2) { - r["alias"] = TEXT(protocol_fields[2]); + r["alias"] = SQL_TEXT(protocol_fields[2]); } // If there is a comment for the service. @@ -66,7 +66,7 @@ QueryData parseEtcProtocolsContent(const std::string& content) { // Removes everything except the comment (parts of the comment). protocol_comment.erase(protocol_comment.begin(), protocol_comment.begin() + 1); - r["comment"] = TEXT(boost::algorithm::join(protocol_comment, " # ")); + r["comment"] = SQL_TEXT(boost::algorithm::join(protocol_comment, " # ")); } results.push_back(r); } diff --git a/osquery/tables/networking/windows/routes.cpp b/osquery/tables/networking/windows/routes.cpp index 8372d6faf69..5f1980bdb11 100644 --- a/osquery/tables/networking/windows/routes.cpp +++ b/osquery/tables/networking/windows/routes.cpp @@ -114,16 +114,16 @@ QueryData genRoutes(QueryContext& context) { auto ipAddress = currentRow.DestinationPrefix.Prefix.Ipv6.sin6_addr; auto gateway = currentRow.NextHop.Ipv6.sin6_addr; - InetNtop(addrFamily, (PVOID)&ipAddress, buffer.data(), buffer.size()); + InetNtopA(addrFamily, (PVOID)&ipAddress, buffer.data(), buffer.size()); r["destination"] = SQL_TEXT(buffer.data()); - InetNtop(addrFamily, (PVOID)&gateway, buffer.data(), buffer.size()); + InetNtopA(addrFamily, (PVOID)&gateway, buffer.data(), buffer.size()); r["gateway"] = SQL_TEXT(buffer.data()); } else if (addrFamily == AF_INET) { std::vector buffer(INET_ADDRSTRLEN); auto ipAddress = currentRow.DestinationPrefix.Prefix.Ipv4.sin_addr; auto gateway = currentRow.NextHop.Ipv4.sin_addr; - InetNtop(addrFamily, (PVOID)&ipAddress, buffer.data(), buffer.size()); + InetNtopA(addrFamily, (PVOID)&ipAddress, buffer.data(), buffer.size()); r["destination"] = SQL_TEXT(buffer.data()); buffer.clear(); @@ -142,7 +142,7 @@ QueryData genRoutes(QueryContext& context) { } } else { interfaceIpAddress = "127.0.0.1"; - InetNtop(addrFamily, (PVOID)&gateway, buffer.data(), buffer.size()); + InetNtopA(addrFamily, (PVOID)&gateway, buffer.data(), buffer.size()); r["gateway"] = SQL_TEXT(buffer.data()); buffer.clear(); } diff --git a/osquery/tables/system/windows/authenticode.cpp b/osquery/tables/system/windows/authenticode.cpp index c8b0b9824db..42d838c4244 100644 --- a/osquery/tables/system/windows/authenticode.cpp +++ b/osquery/tables/system/windows/authenticode.cpp @@ -275,29 +275,29 @@ Status getCertificateInformation(SignatureInformation& signature_info, auto L_GetCertificateDetail = [](std::string& value, PCCERT_CONTEXT certificate_context, DWORD detail) -> bool { - DWORD value_size = CertGetNameString(certificate_context, - CERT_NAME_SIMPLE_DISPLAY_TYPE, - detail, - nullptr, - nullptr, - 0); + DWORD value_size = CertGetNameStringW(certificate_context, + CERT_NAME_SIMPLE_DISPLAY_TYPE, + detail, + nullptr, + nullptr, + 0); if (value_size == 0U || value_size >= 10000) { VLOG(1) << "Invalid certificate field size: " << value_size; return false; } - std::string buffer(static_cast(value_size), 0); + std::wstring buffer(static_cast(value_size), 0); - if (!CertGetNameString(certificate_context, - CERT_NAME_SIMPLE_DISPLAY_TYPE, - detail, - nullptr, - &buffer[0], - value_size)) { + if (!CertGetNameStringW(certificate_context, + CERT_NAME_SIMPLE_DISPLAY_TYPE, + detail, + nullptr, + &buffer[0], + value_size)) { return false; } - value = std::move(buffer); + value = wstringToString(buffer.c_str()); return true; }; diff --git a/osquery/tables/system/windows/chocolatey_packages.cpp b/osquery/tables/system/windows/chocolatey_packages.cpp index c2df1f4f153..1b5b4cf2b5e 100644 --- a/osquery/tables/system/windows/chocolatey_packages.cpp +++ b/osquery/tables/system/windows/chocolatey_packages.cpp @@ -53,7 +53,7 @@ Status genPackage(const fs::path& nuspec, Row& r) { QueryData genChocolateyPackages(QueryContext& context) { QueryData results; - auto chocoEnvInstall = getEnvVar("ChocolateyInstall"); + auto chocoEnvInstall = getEnvVar(L"ChocolateyInstall"); fs::path chocoInstallPath; if (chocoEnvInstall.is_initialized()) { diff --git a/osquery/tables/system/windows/logged_in_users.cpp b/osquery/tables/system/windows/logged_in_users.cpp index e8bac8890c2..6f759181eb5 100644 --- a/osquery/tables/system/windows/logged_in_users.cpp +++ b/osquery/tables/system/windows/logged_in_users.cpp @@ -38,14 +38,14 @@ namespace tables { QueryData genLoggedInUsers(QueryContext& context) { QueryData results; - PWTS_SESSION_INFO_1 pSessionInfo; + PWTS_SESSION_INFO_1W pSessionInfo; unsigned long count; /* * As per the MSDN: * This parameter is reserved. Always set this parameter to one. */ unsigned long level = 1; - auto res = WTSEnumerateSessionsEx( + auto res = WTSEnumerateSessionsExW( WTS_CURRENT_SERVER_HANDLE, &level, 0, &pSessionInfo, &count); if (res == 0) { @@ -72,7 +72,7 @@ QueryData genLoggedInUsers(QueryContext& context) { r["type"] = SQL_TEXT(kSessionStates.at(pSessionInfo[i].State)); r["tty"] = pSessionInfo[i].pSessionName == nullptr ? "" - : pSessionInfo[i].pSessionName; + : wstringToString(pSessionInfo[i].pSessionName); FILETIME utcTime = {0}; unsigned long long unixTime = 0; @@ -83,13 +83,13 @@ QueryData genLoggedInUsers(QueryContext& context) { } r["time"] = INTEGER(unixTime); - char* clientInfo = nullptr; + LPWSTR clientInfo = nullptr; bytesRet = 0; - res = WTSQuerySessionInformation(WTS_CURRENT_SERVER_HANDLE, - pSessionInfo[i].SessionId, - WTSClientInfo, - &clientInfo, - &bytesRet); + res = WTSQuerySessionInformationW(WTS_CURRENT_SERVER_HANDLE, + pSessionInfo[i].SessionId, + WTSClientInfo, + &clientInfo, + &bytesRet); if (res == 0 || clientInfo == nullptr) { VLOG(1) << "Error querying WTS session information (" << GetLastError() << ")"; diff --git a/osquery/tables/system/windows/ntfs_acl_permissions.cpp b/osquery/tables/system/windows/ntfs_acl_permissions.cpp index efc361f0936..bfb9317555a 100644 --- a/osquery/tables/system/windows/ntfs_acl_permissions.cpp +++ b/osquery/tables/system/windows/ntfs_acl_permissions.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -78,30 +79,30 @@ std::string accessPermsToStr(const unsigned long pmask) { } // helper function to get account/group name from trustee -std::string trusteeToStr(const TRUSTEE& trustee) { +std::string trusteeToStr(const TRUSTEEW& trustee) { // Max username length for Windows const unsigned long maxBuffSize = 256; unsigned long sizeOut = maxBuffSize; - char name[maxBuffSize]; - char domain[maxBuffSize]; + WCHAR name[maxBuffSize]; + WCHAR domain[maxBuffSize]; SID_NAME_USE accountType; switch (trustee.TrusteeForm) { case TRUSTEE_IS_SID: { // get the name from the SID PSID psid = trustee.ptstrName; - auto r = LookupAccountSid( + auto r = LookupAccountSidW( nullptr, psid, name, &sizeOut, domain, &sizeOut, &accountType); if (r == FALSE) { VLOG(1) << "LookupAccountSid error: " << GetLastError(); return ""; } else { - return name; + return wstringToString(name); } } case TRUSTEE_IS_NAME: // get the name from ptstrName - return trustee.ptstrName; + return wstringToString(trustee.ptstrName); case TRUSTEE_BAD_FORM: // Indicates a trustee form that is not valid. // https://msdn.microsoft.com/en-us/library/windows/desktop/aa379638(v=vs.85).aspx @@ -109,18 +110,19 @@ std::string trusteeToStr(const TRUSTEE& trustee) { case TRUSTEE_IS_OBJECTS_AND_SID: { // ptstrName member is a pointer to an OBJECTS_AND_SID struct auto psid = reinterpret_cast(trustee.ptstrName)->pSid; - auto r = LookupAccountSid( + auto r = LookupAccountSidW( nullptr, psid, name, &sizeOut, domain, &sizeOut, &accountType); if (r == FALSE) { VLOG(1) << "LookupAccountSid error: " << GetLastError(); return ""; } else { - return name; + return wstringToString(name); } } case TRUSTEE_IS_OBJECTS_AND_NAME: // ptstrName member is a pointer to an OBJECTS_AND_NAME struct - return reinterpret_cast(trustee.ptstrName)->ptstrName; + return wstringToString( + reinterpret_cast(trustee.ptstrName)->ptstrName); default: return ""; } @@ -136,14 +138,14 @@ QueryData genNtfsAclPerms(QueryContext& context) { } // Get a pointer to the existing DACL. PACL dacl = nullptr; - auto result = GetNamedSecurityInfo(pathString.c_str(), - SE_FILE_OBJECT, - DACL_SECURITY_INFORMATION, - nullptr, - nullptr, - &dacl, - nullptr, - nullptr); + auto result = GetNamedSecurityInfoW(stringToWstring(pathString).c_str(), + SE_FILE_OBJECT, + DACL_SECURITY_INFORMATION, + nullptr, + nullptr, + &dacl, + nullptr, + nullptr); if (ERROR_SUCCESS != result) { VLOG(1) << "GetExplicitEnteriesFromAcl Error " << result; continue; @@ -151,8 +153,8 @@ QueryData genNtfsAclPerms(QueryContext& context) { // get list of ACEs from DACL pointer unsigned long aceCount = 0; - PEXPLICIT_ACCESS aceList = nullptr; - result = GetExplicitEntriesFromAcl(dacl, &aceCount, &aceList); + PEXPLICIT_ACCESSW aceList = nullptr; + result = GetExplicitEntriesFromAclW(dacl, &aceCount, &aceList); if (ERROR_SUCCESS != result) { VLOG(1) << "GetExplicitEnteriesFromAcl Error " << result; continue; @@ -178,11 +180,11 @@ QueryData genNtfsAclPerms(QueryContext& context) { : "Unknown"; auto trusteeName = trusteeToStr(aceItem->Trustee); - r["path"] = TEXT(pathString); - r["type"] = TEXT(accessMode); - r["principal"] = TEXT(trusteeName); - r["access"] = TEXT(perms); - r["inherited_from"] = TEXT(inheritedFrom); + r["path"] = pathString; + r["type"] = accessMode; + r["principal"] = trusteeName; + r["access"] = perms; + r["inherited_from"] = inheritedFrom; results.push_back(std::move(r)); } diff --git a/osquery/tables/system/windows/pipes.cpp b/osquery/tables/system/windows/pipes.cpp index e1f07df7ad0..0d65533c7c5 100644 --- a/osquery/tables/system/windows/pipes.cpp +++ b/osquery/tables/system/windows/pipes.cpp @@ -6,6 +6,7 @@ * the LICENSE file found in the root directory of this source tree. */ +#include #include #include @@ -17,11 +18,11 @@ namespace tables { QueryData genPipes(QueryContext& context) { QueryData results; - WIN32_FIND_DATA findFileData; + WIN32_FIND_DATAW findFileData; - std::string pipeSearch = "\\\\.\\pipe\\*"; + std::wstring pipeSearch = L"\\\\.\\pipe\\*"; memset(&findFileData, 0, sizeof(findFileData)); - auto findHandle = FindFirstFileA(pipeSearch.c_str(), &findFileData); + auto findHandle = FindFirstFileW(pipeSearch.c_str(), &findFileData); if (findHandle == INVALID_HANDLE_VALUE) { LOG(INFO) << "Failed to enumerate system pipes"; @@ -31,11 +32,11 @@ QueryData genPipes(QueryContext& context) { do { Row r; - r["name"] = findFileData.cFileName; + r["name"] = wstringToString(findFileData.cFileName); unsigned long pid = 0; - auto pipePath = "\\\\.\\pipe\\" + r["name"]; - auto pipeHandle = CreateFile( + auto pipePath = L"\\\\.\\pipe\\" + std::wstring(findFileData.cFileName); + auto pipeHandle = CreateFileW( pipePath.c_str(), GENERIC_READ, 0, nullptr, OPEN_EXISTING, 0, nullptr); if (pipeHandle == INVALID_HANDLE_VALUE) { results.push_back(r); @@ -69,7 +70,7 @@ QueryData genPipes(QueryContext& context) { results.push_back(r); CloseHandle(pipeHandle); - } while (FindNextFile(findHandle, &findFileData)); + } while (FindNextFileW(findHandle, &findFileData)); FindClose(findHandle); return results; diff --git a/osquery/tables/system/windows/processes.cpp b/osquery/tables/system/windows/processes.cpp index 14fda4edccb..04e7fa6e637 100644 --- a/osquery/tables/system/windows/processes.cpp +++ b/osquery/tables/system/windows/processes.cpp @@ -158,10 +158,10 @@ Status genMemoryMap(unsigned long pid, QueryData& results) { return osquery::join(perms, " | "); }; - MODULEENTRY32 me; + MODULEENTRY32W me; MEMORY_BASIC_INFORMATION mInfo; - me.dwSize = sizeof(MODULEENTRY32); - auto ret = Module32First(modSnap, &me); + me.dwSize = sizeof(MODULEENTRY32W); + auto ret = Module32FirstW(modSnap, &me); while (ret != FALSE) { for (auto p = me.modBaseAddr; VirtualQueryEx(proc, p, &mInfo, sizeof(mInfo)) == sizeof(mInfo) && @@ -182,11 +182,11 @@ Status genMemoryMap(unsigned long pid, QueryData& results) { BIGINT(reinterpret_cast(mInfo.AllocationBase)); r["device"] = "-1"; r["inode"] = INTEGER(-1); - r["path"] = me.szExePath; + r["path"] = wstringToString(me.szExePath); r["pseudo"] = INTEGER(-1); results.push_back(r); } - ret = Module32Next(modSnap, &me); + ret = Module32NextW(modSnap, &me); } CloseHandle(proc); CloseHandle(modSnap); @@ -200,9 +200,9 @@ Status getProcList(std::set& pids) { return Status::failure("Failed to open process snapshot"); } - PROCESSENTRY32 procEntry; - procEntry.dwSize = sizeof(PROCESSENTRY32); - auto ret = Process32First(procSnap, &procEntry); + PROCESSENTRY32W procEntry; + procEntry.dwSize = sizeof(PROCESSENTRY32W); + auto ret = Process32FirstW(procSnap, &procEntry); if (ret == FALSE) { CloseHandle(procSnap); @@ -211,7 +211,7 @@ Status getProcList(std::set& pids) { while (ret != FALSE) { pids.insert(procEntry.th32ProcessID); - ret = Process32Next(procSnap, &procEntry); + ret = Process32NextW(procSnap, &procEntry); } CloseHandle(procSnap); @@ -305,13 +305,13 @@ void getProcessPathInfo(HANDLE& proc, const unsigned long pid, DynamicTableRowHolder& r) { auto out = kMaxPathSize; - std::vector path(kMaxPathSize, 0x0); + std::vector path(kMaxPathSize, 0x0); SecureZeroMemory(path.data(), kMaxPathSize); - auto ret = QueryFullProcessImageName(proc, 0, path.data(), &out); + auto ret = QueryFullProcessImageNameW(proc, 0, path.data(), &out); if (ret != TRUE) { LOG(ERROR) << "Failed to lookup path information for process " << pid; } else { - r["path"] = TEXT(path.data()); + r["path"] = wstringToString(path.data()); } { @@ -323,15 +323,15 @@ void getProcessPathInfo(HANDLE& proc, path.clear(); path.resize(kMaxPathSize, 0x0); if (pid == GetCurrentProcessId()) { - ret = GetModuleFileName(nullptr, path.data(), kMaxPathSize); + ret = GetModuleFileNameW(nullptr, path.data(), kMaxPathSize); } else { - ret = GetModuleFileNameEx(proc, nullptr, path.data(), kMaxPathSize); + ret = GetModuleFileNameExW(proc, nullptr, path.data(), kMaxPathSize); } if (ret == FALSE) { LOG(ERROR) << "Failed to get cwd for " << pid << " with " << GetLastError(); } else { - r["cwd"] = TEXT(path.data()); + r["cwd"] = wstringToString(path.data()); } r["root"] = r["cwd"]; } @@ -451,10 +451,10 @@ TableRows genProcesses(QueryContext& context) { return {}; } - PROCESSENTRY32 proc; - proc.dwSize = sizeof(PROCESSENTRY32); + PROCESSENTRY32W proc; + proc.dwSize = sizeof(PROCESSENTRY32W); - auto ret = Process32First(proc_snap, &proc); + auto ret = Process32FirstW(proc_snap, &proc); if (ret == FALSE) { LOG(ERROR) << "Failed to acquire first process information with " << std::to_string(GetLastError()); @@ -466,14 +466,14 @@ TableRows genProcesses(QueryContext& context) { bool wanted_pid = (pidlist.empty() || pidlist.count(pid) > 0); if (!wanted_pid) { - ret = Process32Next(proc_snap, &proc); + ret = Process32NextW(proc_snap, &proc); continue; } auto r = make_table_row(); r["pid"] = BIGINT(pid); r["parent"] = BIGINT(proc.th32ParentProcessID); - r["name"] = TEXT(proc.szExeFile); + r["name"] = wstringToString(proc.szExeFile); r["threads"] = INTEGER(proc.cntThreads); // Set default values for columns, in the event opening the process fails @@ -516,7 +516,7 @@ TableRows genProcesses(QueryContext& context) { VLOG(1) << "Failed to open handle to process " << proc.th32ProcessID << " with " << GetLastError(); results.push_back(r); - ret = Process32Next(proc_snap, &proc); + ret = Process32NextW(proc_snap, &proc); continue; } @@ -533,11 +533,11 @@ TableRows genProcesses(QueryContext& context) { // handle kNtQueryInformationProcess = reinterpret_cast(GetProcAddress( - GetModuleHandle("ntdll.dll"), "NtQueryInformationProcess")); + GetModuleHandleA("ntdll.dll"), "NtQueryInformationProcess")); kRtlNtStatusToDosError = reinterpret_cast(GetProcAddress( - GetModuleHandle("ntdll.dll"), "RtlNtStatusToDosError")); + GetModuleHandleA("ntdll.dll"), "RtlNtStatusToDosError")); } std::string cmd{""}; @@ -594,7 +594,7 @@ TableRows genProcesses(QueryContext& context) { } results.push_back(r); - ret = Process32Next(proc_snap, &proc); + ret = Process32NextW(proc_snap, &proc); } return results; diff --git a/osquery/tables/system/windows/registry.cpp b/osquery/tables/system/windows/registry.cpp index e0269c45642..59a13519393 100644 --- a/osquery/tables/system/windows/registry.cpp +++ b/osquery/tables/system/windows/registry.cpp @@ -243,8 +243,11 @@ Status queryKey(const std::string& keyPath, QueryData& results) { } HKEY hkey; - auto ret = RegOpenKeyEx( - kRegistryHives.at(hive), TEXT(key.c_str()), 0, KEY_READ, &hkey); + auto ret = RegOpenKeyExW(kRegistryHives.at(hive), + stringToWstring(key).c_str(), + 0, + KEY_READ, + &hkey); if (ret != ERROR_SUCCESS) { return Status(ret, "Failed to open registry handle"); @@ -260,36 +263,36 @@ Status queryKey(const std::string& keyPath, QueryData& results) { DWORD cbMaxValueData; DWORD retCode; FILETIME ftLastWriteTime; - retCode = RegQueryInfoKey(hRegistryHandle.get(), - nullptr, - nullptr, - nullptr, - &cSubKeys, - nullptr, - nullptr, - &cValues, - &cchMaxValueName, - &cbMaxValueData, - nullptr, - &ftLastWriteTime); + retCode = RegQueryInfoKeyW(hRegistryHandle.get(), + nullptr, + nullptr, + nullptr, + &cSubKeys, + nullptr, + nullptr, + &cValues, + &cchMaxValueName, + &cbMaxValueData, + nullptr, + &ftLastWriteTime); if (retCode != ERROR_SUCCESS) { return Status(retCode, "Failed to query registry info for key"); } - auto achKey = std::make_unique(maxKeyLength); + auto achKey = std::make_unique(maxKeyLength); DWORD cbName; // Process registry subkeys if (cSubKeys > 0) { for (DWORD i = 0; i < cSubKeys; i++) { cbName = maxKeyLength; - retCode = RegEnumKeyEx(hRegistryHandle.get(), - i, - achKey.get(), - &cbName, - nullptr, - nullptr, - nullptr, - &ftLastWriteTime); + retCode = RegEnumKeyExW(hRegistryHandle.get(), + i, + achKey.get(), + &cbName, + nullptr, + nullptr, + nullptr, + &ftLastWriteTime); if (retCode != ERROR_SUCCESS) { return Status(retCode, "Failed to enumerate registry key"); } @@ -297,8 +300,8 @@ Status queryKey(const std::string& keyPath, QueryData& results) { Row r; r["key"] = keyPath; r["type"] = "subkey"; - r["name"] = achKey.get(); - r["path"] = keyPath + kRegSep + achKey.get(); + r["name"] = wstringToString(achKey.get()); + r["path"] = keyPath + kRegSep + wstringToString(achKey.get()); r["mtime"] = std::to_string(osquery::filetimeToUnixtime(ftLastWriteTime)); results.push_back(r); } @@ -309,22 +312,22 @@ Status queryKey(const std::string& keyPath, QueryData& results) { } DWORD cchValue = maxKeyLength; - auto achValue = std::make_unique(maxValueName); + auto achValue = std::make_unique(maxValueName); auto bpDataBuff = std::make_unique(cbMaxValueData); // Process registry values for (size_t i = 0; i < cValues; i++) { cchValue = maxValueName; - achValue[0] = '\0'; - - retCode = RegEnumValue(hRegistryHandle.get(), - static_cast(i), - achValue.get(), - &cchValue, - nullptr, - nullptr, - nullptr, - nullptr); + achValue[0] = L'\0'; + + retCode = RegEnumValueW(hRegistryHandle.get(), + static_cast(i), + achValue.get(), + &cchValue, + nullptr, + nullptr, + nullptr, + nullptr); if (retCode != ERROR_SUCCESS) { return Status(retCode, "Failed to enumerate registry values"); } @@ -332,12 +335,12 @@ Status queryKey(const std::string& keyPath, QueryData& results) { DWORD lpData = cbMaxValueData; DWORD lpType; - retCode = RegQueryValueEx(hRegistryHandle.get(), - achValue.get(), - nullptr, - &lpType, - bpDataBuff.get(), - &lpData); + retCode = RegQueryValueExW(hRegistryHandle.get(), + achValue.get(), + nullptr, + &lpType, + bpDataBuff.get(), + &lpData); if (retCode != ERROR_SUCCESS) { return Status(retCode, "Failed to query registry value"); } @@ -351,8 +354,9 @@ Status queryKey(const std::string& keyPath, QueryData& results) { Row r; r["key"] = keyPath; - r["name"] = ((achValue[0] == '\0') ? "(Default)" : achValue.get()); - r["path"] = keyPath + kRegSep + achValue.get(); + r["name"] = ((achValue[0] == L'\0') ? "(Default)" + : wstringToString(achValue.get())); + r["path"] = keyPath + kRegSep + wstringToString(achValue.get()); if (kRegistryTypes.count(lpType) > 0) { r["type"] = kRegistryTypes.at(lpType); } else { @@ -362,21 +366,16 @@ Status queryKey(const std::string& keyPath, QueryData& results) { if (bpDataBuff != nullptr) { /// REG_LINK is a Unicode string, which in Windows is wchar_t - auto regLinkStr = std::make_unique(cbMaxValueData); + std::string regLinkStr; + // std::make_unique(cbMaxValueData); if (lpType == REG_LINK) { - const size_t newSize = cbMaxValueData; - size_t convertedChars = 0; - wcstombs_s(&convertedChars, - regLinkStr.get(), - newSize, - (wchar_t*)bpDataBuff.get(), - _TRUNCATE); + regLinkStr = wstringToString((wchar_t*)bpDataBuff.get()); } std::vector regBinary; std::string data; std::vector multiSzStrs; - auto p = bpDataBuff.get(); + auto p = (wchar_t*)bpDataBuff.get(); switch (lpType) { case REG_FULL_RESOURCE_DESCRIPTOR: @@ -396,15 +395,15 @@ Status queryKey(const std::string& keyPath, QueryData& results) { r["data"] = std::to_string(_byteswap_ulong(*((int*)bpDataBuff.get()))); break; case REG_EXPAND_SZ: - r["data"] = std::string((char*)bpDataBuff.get()); + r["data"] = wstringToString((wchar_t*)bpDataBuff.get()); break; case REG_LINK: - r["data"] = std::string(regLinkStr.get()); + r["data"] = regLinkStr; break; case REG_MULTI_SZ: while (*p != 0x00) { - std::string s((char*)p); - p += s.size() + 1; + std::string s = wstringToString(p); + p += wcslen(p) + 1; multiSzStrs.push_back(s); } r["data"] = boost::algorithm::join(multiSzStrs, ","); @@ -416,7 +415,7 @@ Status queryKey(const std::string& keyPath, QueryData& results) { r["data"] = std::to_string(*((unsigned long long*)bpDataBuff.get())); break; case REG_SZ: - r["data"] = std::string((char*)bpDataBuff.get()); + r["data"] = wstringToString((wchar_t*)bpDataBuff.get()); break; default: r["data"] = ""; diff --git a/osquery/tables/system/windows/scheduled_tasks.cpp b/osquery/tables/system/windows/scheduled_tasks.cpp index bc3604c2252..e3f0d48067a 100644 --- a/osquery/tables/system/windows/scheduled_tasks.cpp +++ b/osquery/tables/system/windows/scheduled_tasks.cpp @@ -198,11 +198,11 @@ QueryData genScheduledTasks(QueryContext& context) { enumerateTasksForFolder("\\", results); // We attempt to derive the location of the tasks folder - auto sysRoot = getEnvVar("SystemRoot"); + auto sysRoot = getEnvVar(L"SystemRoot"); if (!sysRoot.is_initialized()) { return results; } - auto sysTaskPath = *sysRoot + "\\System32\\Tasks"; + auto sysTaskPath = *sysRoot + L"\\System32\\Tasks"; // Then process all tasks in subsequent folders std::vector taskPaths; diff --git a/osquery/tables/system/windows/startup_items.cpp b/osquery/tables/system/windows/startup_items.cpp index 68ec6afe50f..3546cc1b7bb 100644 --- a/osquery/tables/system/windows/startup_items.cpp +++ b/osquery/tables/system/windows/startup_items.cpp @@ -60,9 +60,9 @@ static inline void parseStartupPath(const std::string& path, Row& r) { if (path.find('\"') == std::string::npos) { r["path"] = path; } else { - boost::tokenizer> tokens( + boost::tokenizer> tokens( path, - boost::escaped_list_separator( + boost::escaped_list_separator( std::string(""), std::string(" "), std::string("\"\'"))); for (auto&& tok = tokens.begin(); tok != tokens.end(); ++tok) { if (tok == tokens.begin()) { diff --git a/osquery/tables/system/windows/windows_crashes.cpp b/osquery/tables/system/windows/windows_crashes.cpp index a13eff8b8bf..274146ebf9a 100644 --- a/osquery/tables/system/windows/windows_crashes.cpp +++ b/osquery/tables/system/windows/windows_crashes.cpp @@ -622,8 +622,10 @@ QueryData genCrashLogs(QueryContext& context) { if (!dumpFolderResults.rows().empty()) { dumpFolderLocation = dumpFolderResults.rows()[0].at("data"); } else { - auto tempDumpLoc = getEnvVar("TMP"); - dumpFolderLocation = tempDumpLoc.is_initialized() ? *tempDumpLoc : ""; + auto tempDumpLoc = getEnvVar(L"TMP"); + dumpFolderLocation = tempDumpLoc.is_initialized() + ? wstringToString(tempDumpLoc.get().c_str()) + : ""; } if (!fs::exists(dumpFolderLocation) || diff --git a/osquery/utils/aws/tests/aws_util_tests.cpp b/osquery/utils/aws/tests/aws_util_tests.cpp index a756a45a269..6ca0f4491de 100644 --- a/osquery/utils/aws/tests/aws_util_tests.cpp +++ b/osquery/utils/aws/tests/aws_util_tests.cpp @@ -15,6 +15,7 @@ #include #include +#include #include namespace osquery { @@ -45,11 +46,12 @@ class AwsUtilTests : public testing::Test { TEST_F(AwsUtilTests, test_get_credentials) { // Set a good path for the credentials file auto const profile_path = getTestConfigDirectory() / "/aws/credentials"; - setEnvVar(kAwsProfileFileEnvVar, profile_path.string()); + setEnvVar(stringToWstring(kAwsProfileFileEnvVar), + stringToWstring(profile_path.string())); // Clear any values for the other AWS env vars - unsetEnvVar(kAwsAccessKeyEnvVar); - unsetEnvVar(kAwsSecretKeyEnvVar); + unsetEnvVar(stringToWstring(kAwsAccessKeyEnvVar)); + unsetEnvVar(stringToWstring(kAwsSecretKeyEnvVar)); Aws::Auth::AWSCredentials credentials("", ""); @@ -100,8 +102,8 @@ TEST_F(AwsUtilTests, test_get_credentials) { FLAGS_aws_access_key_id = ""; FLAGS_aws_secret_access_key = ""; - setEnvVar(kAwsAccessKeyEnvVar, "ENV_ACCESS_KEY_ID"); - setEnvVar(kAwsSecretKeyEnvVar, "env_secret_key"); + setEnvVar(stringToWstring(kAwsAccessKeyEnvVar), L"ENV_ACCESS_KEY_ID"); + setEnvVar(stringToWstring(kAwsSecretKeyEnvVar), L"env_secret_key"); { // Now env variables should be the primary source OsqueryAWSCredentialsProviderChain provider; @@ -153,7 +155,7 @@ TEST_F(AwsUtilTests, test_get_region) { // Test no credential file, should default to us-east-1 auto profile_path = getTestConfigDirectory() / "credentials"; - setEnvVar(kAwsProfileFileEnvVar, profile_path.string()); + setEnvVar(stringToWstring(kAwsProfileFileEnvVar), stringToWstring(profile_path.string())); ASSERT_EQ(Status(0), getAWSRegion(region)); ASSERT_EQ(std::string(Aws::Region::US_EAST_1), region); @@ -163,13 +165,15 @@ TEST_F(AwsUtilTests, test_get_region) { // Set an invalid path for the credentials file with a profile name // provided, profile_path = getTestConfigDirectory() / "credentials"; - setEnvVar(kAwsProfileFileEnvVar, profile_path.string()); + setEnvVar(stringToWstring(kAwsProfileFileEnvVar), + stringToWstring(profile_path.string())); FLAGS_aws_profile_name = "test"; ASSERT_FALSE(getAWSRegion(region).ok()); // Set a valid path for the credentials file with profile name. profile_path = getTestConfigDirectory() / "aws/credentials"; - setEnvVar(kAwsProfileFileEnvVar, profile_path.string()); + setEnvVar(stringToWstring(kAwsProfileFileEnvVar), + stringToWstring(profile_path.string())); FLAGS_aws_profile_name = "test"; ASSERT_EQ(Status(0), getAWSRegion(region)); ASSERT_EQ(std::string(Aws::Region::EU_CENTRAL_1), region); diff --git a/osquery/utils/system/env.h b/osquery/utils/system/env.h index 5a5e62d707b..ebb26943406 100644 --- a/osquery/utils/system/env.h +++ b/osquery/utils/system/env.h @@ -15,16 +15,16 @@ namespace osquery { /// Set the environment variable name with value value. -bool setEnvVar(const std::string& name, const std::string& value); +bool setEnvVar(const std::wstring& name, const std::wstring& value); /// Unsets the environment variable specified by name. -bool unsetEnvVar(const std::string& name); +bool unsetEnvVar(const std::wstring& name); /** * @brief Returns the value of the specified environment variable name. * * If the environment variable does not exist, boost::none is returned. */ -boost::optional getEnvVar(const std::string& name); +boost::optional getEnvVar(const std::wstring& name); } // namespace osquery diff --git a/osquery/utils/system/errno.h b/osquery/utils/system/errno.h index 39ef412f05c..2c3264e52fc 100644 --- a/osquery/utils/system/errno.h +++ b/osquery/utils/system/errno.h @@ -26,7 +26,7 @@ std::string platformStrerr(int errnum); #ifdef WIN32 /// Converts a Windows error (winerror.h/GetLastError()) to a string -Status getWindowsErrorDescription(std::string& error_message, DWORD error_id); +Status getWindowsErrorDescription(std::wstring& error_message, DWORD error_id); #endif } // namespace osquery diff --git a/osquery/utils/system/windows/env.cpp b/osquery/utils/system/windows/env.cpp index 0ccd0491256..caa68fbb389 100644 --- a/osquery/utils/system/windows/env.cpp +++ b/osquery/utils/system/windows/env.cpp @@ -17,21 +17,21 @@ namespace osquery { -bool setEnvVar(const std::string& name, const std::string& value) { - return (::SetEnvironmentVariableA(name.c_str(), value.c_str()) == TRUE); +bool setEnvVar(const std::wstring& name, const std::wstring& value) { + return (::SetEnvironmentVariableW(name.c_str(), value.c_str()) == TRUE); } -bool unsetEnvVar(const std::string& name) { - return (::SetEnvironmentVariableA(name.c_str(), nullptr) == TRUE); +bool unsetEnvVar(const std::wstring& name) { + return (::SetEnvironmentVariableW(name.c_str(), nullptr) == TRUE); } -boost::optional getEnvVar(const std::string& name) { +boost::optional getEnvVar(const std::wstring& name) { const auto kInitialBufferSize = 1024; - std::vector buf; + std::vector buf; buf.assign(kInitialBufferSize, '\0'); auto value_len = - ::GetEnvironmentVariableA(name.c_str(), buf.data(), kInitialBufferSize); + ::GetEnvironmentVariableW(name.c_str(), buf.data(), kInitialBufferSize); if (value_len == 0) { return boost::none; } @@ -42,7 +42,7 @@ boost::optional getEnvVar(const std::string& name) { // returned size is greater than what we expect. if (value_len > kInitialBufferSize) { buf.assign(value_len, '\0'); - value_len = ::GetEnvironmentVariableA(name.c_str(), buf.data(), value_len); + value_len = ::GetEnvironmentVariableW(name.c_str(), buf.data(), value_len); if (value_len == 0 || value_len > buf.size()) { // The size returned is greater than the size we expected. Currently, we // will not deal with this scenario and just return as if an error has @@ -51,7 +51,7 @@ boost::optional getEnvVar(const std::string& name) { } } - return std::string(buf.data(), value_len); + return std::wstring(buf.data(), value_len); } } // namespace osquery diff --git a/osquery/utils/system/windows/errno.cpp b/osquery/utils/system/windows/errno.cpp index ef9a7fbdd62..1310defbac4 100644 --- a/osquery/utils/system/windows/errno.cpp +++ b/osquery/utils/system/windows/errno.cpp @@ -28,17 +28,17 @@ std::string platformStrerr(int errnum) { return std::string(buffer.data()); } -Status getWindowsErrorDescription(std::string& error_message, DWORD error_id) { +Status getWindowsErrorDescription(std::wstring& error_message, DWORD error_id) { error_message.clear(); - LPSTR buffer = nullptr; + LPWSTR buffer = nullptr; - auto message_size = FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | + auto message_size = FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, nullptr, error_id, kWindowsLanguageId, - reinterpret_cast(&buffer), + reinterpret_cast(&buffer), 0, nullptr); @@ -49,7 +49,7 @@ Status getWindowsErrorDescription(std::string& error_message, DWORD error_id) { std::to_string(error_id)); } - error_message.assign(buffer, static_cast(message_size)); + error_message = buffer; LocalFree(buffer); return Status(0); diff --git a/tests/integration/tables/chrome_extensions.cpp b/tests/integration/tables/chrome_extensions.cpp index 453262410c3..15bfab2a5c3 100644 --- a/tests/integration/tables/chrome_extensions.cpp +++ b/tests/integration/tables/chrome_extensions.cpp @@ -32,6 +32,7 @@ TEST_F(chromeExtensions, test_sanity) { {"locale", NormalType}, {"update_url", NonEmptyString}, {"author", NormalType}, + {"optional_permissions", NormalType}, {"persistent", IntType}, {"path", NonEmptyString}, {"permissions", NormalType}, From 8fe8326f2ed4dce42120f1f41a2b8c0d0484ba7d Mon Sep 17 00:00:00 2001 From: Ateeq Sharfuddin Date: Sun, 19 Jan 2020 19:32:46 -0500 Subject: [PATCH 02/16] add SQL_TEXT, code style check, and revert EnvVar args to std::string --- osquery/config/tests/test_utils.cpp | 6 +- osquery/core/tests/windows/wmi_tests.cpp | 4 +- osquery/devtools/shell.cpp | 4 +- osquery/filesystem/tests/filesystem.cpp | 7 +- osquery/filesystem/windows/fileops.cpp | 9 +- osquery/process/windows/process.cpp | 26 ++--- osquery/remote/enroll/enroll.cpp | 6 +- .../system/windows/ntfs_acl_permissions.cpp | 10 +- osquery/tables/system/windows/processes.cpp | 6 +- .../tables/system/windows/windows_crashes.cpp | 4 +- osquery/utils/aws/tests/aws_util_tests.cpp | 19 ++-- osquery/utils/system/windows/env.cpp | 106 ++++++++++++++---- 12 files changed, 129 insertions(+), 78 deletions(-) diff --git a/osquery/config/tests/test_utils.cpp b/osquery/config/tests/test_utils.cpp index 95403f1bd25..0537cae67b6 100644 --- a/osquery/config/tests/test_utils.cpp +++ b/osquery/config/tests/test_utils.cpp @@ -24,12 +24,10 @@ namespace { namespace fs = boost::filesystem; fs::path getConfDirPathImpl() { - std::wstring kEnvVarName = L"TEST_CONF_FILES_DIR"; + std::string kEnvVarName = "TEST_CONF_FILES_DIR"; auto const value_opt = osquery::getEnvVar(kEnvVarName); EXPECT_TRUE(static_cast(value_opt)) - << "Env var " - << boost::io::quoted(osquery::wstringToString(kEnvVarName.c_str())) - << " was not found, " + << "Env var " << boost::io::quoted(kEnvVarName) << " was not found, " << " looks like cxx_test argument 'env' is not set up."; return fs::path(value_opt.get()); } diff --git a/osquery/core/tests/windows/wmi_tests.cpp b/osquery/core/tests/windows/wmi_tests.cpp index 1abd7eb3c86..a646d52ac84 100644 --- a/osquery/core/tests/windows/wmi_tests.cpp +++ b/osquery/core/tests/windows/wmi_tests.cpp @@ -32,12 +32,12 @@ class WmiTests : public testing::Test { }; TEST_F(WmiTests, test_methodcall_inparams) { - auto windir = getEnvVar(L"WINDIR"); + auto windir = getEnvVar("WINDIR"); EXPECT_TRUE(windir); std::stringstream ss; ss << "SELECT * FROM Win32_Directory WHERE Name = \"" - << wstringToString(windir.get().c_str()) << "\""; + << *windir << "\""; auto query = ss.str(); diff --git a/osquery/devtools/shell.cpp b/osquery/devtools/shell.cpp index a00859c7196..25dcacde123 100644 --- a/osquery/devtools/shell.cpp +++ b/osquery/devtools/shell.cpp @@ -1641,9 +1641,9 @@ static void main_init(struct callback_data* data) { sqlite3_config(SQLITE_CONFIG_URI, 1); sqlite3_config(SQLITE_CONFIG_LOG, shellLog, data); - auto term = osquery::getEnvVar(L"TERM"); + auto term = osquery::getEnvVar("TERM"); if (term.is_initialized() && - (*term).find(L"xterm-256color") != std::wstring::npos) { + (*term).find("xterm-256color") != std::wstring::npos) { sqlite3_snprintf( sizeof(mainPrompt), mainPrompt, "\033[38;5;147mosquery> \033[0m"); sqlite3_snprintf(sizeof(continuePrompt), diff --git a/osquery/filesystem/tests/filesystem.cpp b/osquery/filesystem/tests/filesystem.cpp index 8604b034198..33a1b7a11b0 100644 --- a/osquery/filesystem/tests/filesystem.cpp +++ b/osquery/filesystem/tests/filesystem.cpp @@ -56,11 +56,8 @@ class FilesystemTests : public testing::Test { tmp_path_ = fs::temp_directory_path().string(); line_ending_ = "\r\n"; - auto raw_drive = getEnvVar(L"SystemDrive"); - system_root_ = - (raw_drive.is_initialized() ? wstringToString(raw_drive.get().c_str()) - : "") + - "\\"; + auto raw_drive = getEnvVar("SystemDrive"); + system_root_ = (raw_drive.is_initialized() ? *raw_drive : "") + "\\"; } else { etc_hosts_path_ = "/etc/hosts"; etc_path_ = "/etc"; diff --git a/osquery/filesystem/windows/fileops.cpp b/osquery/filesystem/windows/fileops.cpp index db6b80bdb37..de37ea70b32 100644 --- a/osquery/filesystem/windows/fileops.cpp +++ b/osquery/filesystem/windows/fileops.cpp @@ -1306,10 +1306,9 @@ std::vector platformGlob(const std::string& find_path) { */ if (find_path.size() >= 2 && find_path[0] == '~' && (find_path[1] == '/' || find_path[1] == '\\')) { - auto homedir = getEnvVar(L"USERPROFILE"); + auto homedir = getEnvVar("USERPROFILE"); if (homedir.is_initialized()) { - std::wstring whomedir = *homedir; - full_path = fs::path(wstringToString(whomedir.c_str())) / find_path.substr(2); + full_path = fs::path(*homedir) / find_path.substr(2); } } @@ -1423,9 +1422,9 @@ std::vector platformGlob(const std::string& find_path) { boost::optional getHomeDirectory() { std::vector profile(MAX_PATH); - auto value = getEnvVar(L"USERPROFILE"); + auto value = getEnvVar("USERPROFILE"); if (value.is_initialized()) { - return wstringToString(value.get().c_str()); + return *value; } else if (SUCCEEDED(::SHGetFolderPathW( nullptr, CSIDL_PROFILE, nullptr, 0, profile.data()))) { return wstringToString(profile.data()); diff --git a/osquery/process/windows/process.cpp b/osquery/process/windows/process.cpp index 44966f4e668..631c4732855 100644 --- a/osquery/process/windows/process.cpp +++ b/osquery/process/windows/process.cpp @@ -123,7 +123,7 @@ int PlatformProcess::getCurrentPid() { } std::shared_ptr PlatformProcess::getLauncherProcess() { - auto launcher_handle = getEnvVar(L"OSQUERY_LAUNCHER"); + auto launcher_handle = getEnvVar("OSQUERY_LAUNCHER"); if (!launcher_handle) { return std::make_shared(); } @@ -184,8 +184,8 @@ std::shared_ptr PlatformProcess::launchWorker( // OSQUERY_LAUNCHER. OSQUERY_LAUNCHER stores the string form of a HANDLE to // the current process. This is mostly used for detecting the death of the // launcher process in WatcherWatcherRunner::start - if (!setEnvVar(L"OSQUERY_WORKER", L"1") || - !setEnvVar(L"OSQUERY_LAUNCHER", stringToWstring(handle))) { + if (!setEnvVar("OSQUERY_WORKER", "1") || + !setEnvVar("OSQUERY_LAUNCHER", handle)) { ::CloseHandle(hLauncherProcess); return std::shared_ptr(); @@ -229,8 +229,8 @@ std::shared_ptr PlatformProcess::launchWorker( nullptr, &si, &pi); - unsetEnvVar(L"OSQUERY_WORKER"); - unsetEnvVar(L"OSQUERY_LAUNCHER"); + unsetEnvVar("OSQUERY_WORKER"); + unsetEnvVar("OSQUERY_LAUNCHER"); ::CloseHandle(hLauncherProcess); if (!status) { @@ -277,7 +277,7 @@ std::shared_ptr PlatformProcess::launchExtension( // 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 // this is never used elsewhere in the core. - if (!setEnvVar(L"OSQUERY_EXTENSION", L"1")) { + if (!setEnvVar("OSQUERY_EXTENSION", "1")) { return std::shared_ptr(); } @@ -298,7 +298,7 @@ std::shared_ptr PlatformProcess::launchExtension( nullptr, &si, &pi); - unsetEnvVar(L"OSQUERY_EXTENSION"); + unsetEnvVar("OSQUERY_EXTENSION"); if (!status) { return std::shared_ptr(); @@ -317,13 +317,13 @@ std::shared_ptr PlatformProcess::launchTestPythonScript( STARTUPINFOW si = {0}; PROCESS_INFORMATION pi = {nullptr}; - auto argv = L"python " + stringToWstring(args); + auto argv = "python " + args; std::vector mutable_argv(argv.begin(), argv.end()); - mutable_argv.push_back(L'\0'); + mutable_argv.push_back('\0'); si.cb = sizeof(si); - auto pythonEnv = getEnvVar(L"OSQUERY_PYTHON_PATH"); - std::wstring pythonPath; + auto pythonEnv = getEnvVar("OSQUERY_PYTHON_PATH"); + std::string pythonPath; if (pythonEnv.is_initialized()) { pythonPath = *pythonEnv; } @@ -331,10 +331,10 @@ std::shared_ptr PlatformProcess::launchTestPythonScript( // Python is installed at this location if the provisioning script is used. // This path should work regardless of the existence of the SystemDrive // environment variable. - pythonPath += L"\\python.exe"; + pythonPath += "\\python.exe"; std::shared_ptr process; - if (::CreateProcessW(pythonPath.c_str(), + if (::CreateProcessW(stringToWstring(pythonPath).c_str(), mutable_argv.data(), nullptr, nullptr, diff --git a/osquery/remote/enroll/enroll.cpp b/osquery/remote/enroll/enroll.cpp index 5ce60491f7a..2a09b240492 100644 --- a/osquery/remote/enroll/enroll.cpp +++ b/osquery/remote/enroll/enroll.cpp @@ -98,11 +98,9 @@ const std::string getEnrollSecret() { readFile(FLAGS_enroll_secret_path, enrollment_secret); boost::trim(enrollment_secret); } else { - std::wstring wFLAGS_enroll_secret_env = - stringToWstring(FLAGS_enroll_secret_env); - auto env_secret = getEnvVar(wFLAGS_enroll_secret_env); + auto env_secret = getEnvVar(FLAGS_enroll_secret_env); if (env_secret.is_initialized()) { - enrollment_secret = wstringToString(env_secret.get().c_str()); + enrollment_secret = *env_secret; } } diff --git a/osquery/tables/system/windows/ntfs_acl_permissions.cpp b/osquery/tables/system/windows/ntfs_acl_permissions.cpp index bfb9317555a..f3e4ced5fd5 100644 --- a/osquery/tables/system/windows/ntfs_acl_permissions.cpp +++ b/osquery/tables/system/windows/ntfs_acl_permissions.cpp @@ -180,11 +180,11 @@ QueryData genNtfsAclPerms(QueryContext& context) { : "Unknown"; auto trusteeName = trusteeToStr(aceItem->Trustee); - r["path"] = pathString; - r["type"] = accessMode; - r["principal"] = trusteeName; - r["access"] = perms; - r["inherited_from"] = inheritedFrom; + r["path"] = SQL_TEXT(pathString); + r["type"] = SQL_TEXT(accessMode); + r["principal"] = SQL_TEXT(trusteeName); + r["access"] = SQL_TEXT(perms); + r["inherited_from"] = SQL_TEXT(inheritedFrom); results.push_back(std::move(r)); } diff --git a/osquery/tables/system/windows/processes.cpp b/osquery/tables/system/windows/processes.cpp index 04e7fa6e637..ddd99dddf09 100644 --- a/osquery/tables/system/windows/processes.cpp +++ b/osquery/tables/system/windows/processes.cpp @@ -311,7 +311,7 @@ void getProcessPathInfo(HANDLE& proc, if (ret != TRUE) { LOG(ERROR) << "Failed to lookup path information for process " << pid; } else { - r["path"] = wstringToString(path.data()); + r["path"] = SQL_TEXT(wstringToString(path.data())); } { @@ -331,7 +331,7 @@ void getProcessPathInfo(HANDLE& proc, if (ret == FALSE) { LOG(ERROR) << "Failed to get cwd for " << pid << " with " << GetLastError(); } else { - r["cwd"] = wstringToString(path.data()); + r["cwd"] = SQL_TEXT(wstringToString(path.data())); } r["root"] = r["cwd"]; } @@ -473,7 +473,7 @@ TableRows genProcesses(QueryContext& context) { auto r = make_table_row(); r["pid"] = BIGINT(pid); r["parent"] = BIGINT(proc.th32ParentProcessID); - r["name"] = wstringToString(proc.szExeFile); + r["name"] = SQL_TEXT(wstringToString(proc.szExeFile)); r["threads"] = INTEGER(proc.cntThreads); // Set default values for columns, in the event opening the process fails diff --git a/osquery/tables/system/windows/windows_crashes.cpp b/osquery/tables/system/windows/windows_crashes.cpp index 274146ebf9a..558b3959964 100644 --- a/osquery/tables/system/windows/windows_crashes.cpp +++ b/osquery/tables/system/windows/windows_crashes.cpp @@ -622,9 +622,9 @@ QueryData genCrashLogs(QueryContext& context) { if (!dumpFolderResults.rows().empty()) { dumpFolderLocation = dumpFolderResults.rows()[0].at("data"); } else { - auto tempDumpLoc = getEnvVar(L"TMP"); + auto tempDumpLoc = getEnvVar("TMP"); dumpFolderLocation = tempDumpLoc.is_initialized() - ? wstringToString(tempDumpLoc.get().c_str()) + ? *tempDumpLoc : ""; } diff --git a/osquery/utils/aws/tests/aws_util_tests.cpp b/osquery/utils/aws/tests/aws_util_tests.cpp index 6ca0f4491de..e932dec8ea1 100644 --- a/osquery/utils/aws/tests/aws_util_tests.cpp +++ b/osquery/utils/aws/tests/aws_util_tests.cpp @@ -46,12 +46,11 @@ class AwsUtilTests : public testing::Test { TEST_F(AwsUtilTests, test_get_credentials) { // Set a good path for the credentials file auto const profile_path = getTestConfigDirectory() / "/aws/credentials"; - setEnvVar(stringToWstring(kAwsProfileFileEnvVar), - stringToWstring(profile_path.string())); + setEnvVar(kAwsProfileFileEnvVar, profile_path.string()); // Clear any values for the other AWS env vars - unsetEnvVar(stringToWstring(kAwsAccessKeyEnvVar)); - unsetEnvVar(stringToWstring(kAwsSecretKeyEnvVar)); + unsetEnvVar(kAwsAccessKeyEnvVar); + unsetEnvVar(kAwsSecretKeyEnvVar); Aws::Auth::AWSCredentials credentials("", ""); @@ -102,8 +101,8 @@ TEST_F(AwsUtilTests, test_get_credentials) { FLAGS_aws_access_key_id = ""; FLAGS_aws_secret_access_key = ""; - setEnvVar(stringToWstring(kAwsAccessKeyEnvVar), L"ENV_ACCESS_KEY_ID"); - setEnvVar(stringToWstring(kAwsSecretKeyEnvVar), L"env_secret_key"); + setEnvVar(kAwsAccessKeyEnvVar, "ENV_ACCESS_KEY_ID"); + setEnvVar(kAwsSecretKeyEnvVar, "env_secret_key"); { // Now env variables should be the primary source OsqueryAWSCredentialsProviderChain provider; @@ -155,7 +154,7 @@ TEST_F(AwsUtilTests, test_get_region) { // Test no credential file, should default to us-east-1 auto profile_path = getTestConfigDirectory() / "credentials"; - setEnvVar(stringToWstring(kAwsProfileFileEnvVar), stringToWstring(profile_path.string())); + setEnvVar(kAwsProfileFileEnvVar, profile_path.string()); ASSERT_EQ(Status(0), getAWSRegion(region)); ASSERT_EQ(std::string(Aws::Region::US_EAST_1), region); @@ -165,15 +164,13 @@ TEST_F(AwsUtilTests, test_get_region) { // Set an invalid path for the credentials file with a profile name // provided, profile_path = getTestConfigDirectory() / "credentials"; - setEnvVar(stringToWstring(kAwsProfileFileEnvVar), - stringToWstring(profile_path.string())); + setEnvVar(kAwsProfileFileEnvVar, profile_path.string()); FLAGS_aws_profile_name = "test"; ASSERT_FALSE(getAWSRegion(region).ok()); // Set a valid path for the credentials file with profile name. profile_path = getTestConfigDirectory() / "aws/credentials"; - setEnvVar(stringToWstring(kAwsProfileFileEnvVar), - stringToWstring(profile_path.string())); + setEnvVar(kAwsProfileFileEnvVar, profile_path.string()); FLAGS_aws_profile_name = "test"; ASSERT_EQ(Status(0), getAWSRegion(region)); ASSERT_EQ(std::string(Aws::Region::EU_CENTRAL_1), region); diff --git a/osquery/utils/system/windows/env.cpp b/osquery/utils/system/windows/env.cpp index caa68fbb389..dbda9c18970 100644 --- a/osquery/utils/system/windows/env.cpp +++ b/osquery/utils/system/windows/env.cpp @@ -17,41 +17,103 @@ namespace osquery { -bool setEnvVar(const std::wstring& name, const std::wstring& value) { - return (::SetEnvironmentVariableW(name.c_str(), value.c_str()) == TRUE); +bool setEnvVar(const std::string& name, const std::string& value) { + bool status = false; + std::vector widename; + widename.assign((name.length() + 1) * 2, '\0'); + + std::vector widevalue; + widevalue.assign((value.length() + 1) * 2, '\0'); + + if (0 != MultiByteToWideChar(CP_UTF8, + 0, + name.c_str(), + -1, + widename.data(), + (name.length() + 1) * 2)) { + if (0 != MultiByteToWideChar(CP_UTF8, + 0, + value.c_str(), + -1, + widevalue.data(), + (value.length() + 1) * 2)) { + status = (::SetEnvironmentVariableW(widename.data(), widevalue.data()) == + TRUE); + } + } + + return status; } -bool unsetEnvVar(const std::wstring& name) { - return (::SetEnvironmentVariableW(name.c_str(), nullptr) == TRUE); +bool unsetEnvVar(const std::string& name) { + bool status = false; + std::vector widename; + widename.assign((name.length() + 1) * 2, '\0'); + + if (0 != MultiByteToWideChar(CP_UTF8, + 0, + name.c_str(), + -1, + widename.data(), + (name.length() + 1) * 2)) { + status = (::SetEnvironmentVariableW(widename.data(), nullptr) == TRUE); + } + + return status; } -boost::optional getEnvVar(const std::wstring& name) { +boost::optional getEnvVar(const std::string& name) { const auto kInitialBufferSize = 1024; std::vector buf; buf.assign(kInitialBufferSize, '\0'); - auto value_len = - ::GetEnvironmentVariableW(name.c_str(), buf.data(), kInitialBufferSize); - if (value_len == 0) { - return boost::none; - } + std::vector widename; + widename.assign((name.length() + 1) * 2, '\0'); - // It is always possible that between the first GetEnvironmentVariableA call - // and this one, a change was made to our target environment variable that - // altered the size. Currently, we ignore this scenario and fail if the - // returned size is greater than what we expect. - if (value_len > kInitialBufferSize) { - buf.assign(value_len, '\0'); - value_len = ::GetEnvironmentVariableW(name.c_str(), buf.data(), value_len); - if (value_len == 0 || value_len > buf.size()) { - // The size returned is greater than the size we expected. Currently, we - // will not deal with this scenario and just return as if an error has - // occurred. + std::vector narrowvalue; + + if (0 != MultiByteToWideChar(CP_UTF8, + 0, + name.c_str(), + -1, + widename.data(), + (name.length() + 1) * 2)) { + auto value_len = ::GetEnvironmentVariableW( + widename.data(), buf.data(), kInitialBufferSize); + if (value_len == 0) { return boost::none; } + + // It is always possible that between the first GetEnvironmentVariableA call + // and this one, a change was made to our target environment variable that + // altered the size. Currently, we ignore this scenario and fail if the + // returned size is greater than what we expect. + if (value_len > kInitialBufferSize) { + buf.assign(value_len, '\0'); + value_len = + ::GetEnvironmentVariableW(widename.data(), buf.data(), value_len); + if (value_len == 0 || value_len > buf.size()) { + // The size returned is greater than the size we expected. Currently, we + // will not deal with this scenario and just return as if an error has + // occurred. + return boost::none; + } + } + + narrowvalue.assign((value_len + 1) * 4, '\0'); + WideCharToMultiByte(CP_UTF8, + 0, + buf.data(), + -1, + narrowvalue.data(), + value_len + 1 * 4, + 0, + nullptr); + + return narrowvalue.data(); } - return std::wstring(buf.data(), value_len); + return std::string(); } } // namespace osquery From 9fef4c651f59df843523038a1d64652e871fab7d Mon Sep 17 00:00:00 2001 From: Ateeq Sharfuddin Date: Sun, 19 Jan 2020 19:41:20 -0500 Subject: [PATCH 03/16] adding missing updates --- osquery/core/init.cpp | 2 +- osquery/core/tests/process_tests.cpp | 16 ++++++++-------- osquery/core/tests/windows/wmi_tests.cpp | 4 +--- osquery/core/watcher.cpp | 4 ++-- osquery/devtools/printer.cpp | 4 ++-- osquery/devtools/shell.cpp | 2 +- 6 files changed, 15 insertions(+), 17 deletions(-) diff --git a/osquery/core/init.cpp b/osquery/core/init.cpp index 12dc28545dd..e7f9c777e99 100644 --- a/osquery/core/init.cpp +++ b/osquery/core/init.cpp @@ -97,7 +97,7 @@ CLI_FLAG(bool, namespace { extern "C" { static inline bool hasWorkerVariable() { - return ::osquery::getEnvVar(L"OSQUERY_WORKER").is_initialized(); + return ::osquery::getEnvVar("OSQUERY_WORKER").is_initialized(); } volatile std::sig_atomic_t kHandledSignal{0}; diff --git a/osquery/core/tests/process_tests.cpp b/osquery/core/tests/process_tests.cpp index 4bee2f7f649..5f0d3a3446a 100644 --- a/osquery/core/tests/process_tests.cpp +++ b/osquery/core/tests/process_tests.cpp @@ -153,20 +153,20 @@ TEST_F(ProcessTests, test_getpid) { } TEST_F(ProcessTests, test_envVar) { - auto val = getEnvVar(L"GTEST_OSQUERY"); + auto val = getEnvVar("GTEST_OSQUERY"); EXPECT_FALSE(val); EXPECT_FALSE(val.is_initialized()); - EXPECT_TRUE(setEnvVar(L"GTEST_OSQUERY", L"true")); + EXPECT_TRUE(setEnvVar("GTEST_OSQUERY", "true")); - val = getEnvVar(L"GTEST_OSQUERY"); + val = getEnvVar("GTEST_OSQUERY"); EXPECT_FALSE(!val); EXPECT_TRUE(val.is_initialized()); - EXPECT_EQ(*val, L"true"); + EXPECT_EQ(*val, "true"); - EXPECT_TRUE(unsetEnvVar(L"GTEST_OSQUERY")); + EXPECT_TRUE(unsetEnvVar("GTEST_OSQUERY")); - val = getEnvVar(L"GTEST_OSQUERY"); + val = getEnvVar("GTEST_OSQUERY"); EXPECT_FALSE(val); EXPECT_FALSE(val.is_initialized()); } @@ -302,9 +302,9 @@ int main(int argc, char* argv[]) { osquery::kExpectedExtensionArgs[0] = argv[0]; osquery::kExpectedWorkerArgs[0] = argv[0]; - if (auto val = osquery::getEnvVar(L"OSQUERY_WORKER")) { + if (auto val = osquery::getEnvVar("OSQUERY_WORKER")) { return workerMain(argc, argv); - } else if ((val = osquery::getEnvVar(L"OSQUERY_EXTENSION"))) { + } else if ((val = osquery::getEnvVar("OSQUERY_EXTENSION"))) { return extensionMain(argc, argv); } diff --git a/osquery/core/tests/windows/wmi_tests.cpp b/osquery/core/tests/windows/wmi_tests.cpp index a646d52ac84..1e0640b19da 100644 --- a/osquery/core/tests/windows/wmi_tests.cpp +++ b/osquery/core/tests/windows/wmi_tests.cpp @@ -17,7 +17,6 @@ #include #include #include -#include #include #include "osquery/core/windows/wmi.h" @@ -36,8 +35,7 @@ TEST_F(WmiTests, test_methodcall_inparams) { EXPECT_TRUE(windir); std::stringstream ss; - ss << "SELECT * FROM Win32_Directory WHERE Name = \"" - << *windir << "\""; + ss << "SELECT * FROM Win32_Directory WHERE Name = \"" << *windir << "\""; auto query = ss.str(); diff --git a/osquery/core/watcher.cpp b/osquery/core/watcher.cpp index 7abd0f8dd66..49d439aabb1 100644 --- a/osquery/core/watcher.cpp +++ b/osquery/core/watcher.cpp @@ -182,7 +182,7 @@ bool Watcher::hasManagedExtensions() const { // Setting this counter to 0 will prevent the worker from waiting for missing // dependent config plugins. Otherwise, its existence, will cause a worker to // wait for missing plugins to broadcast from managed extensions. - return getEnvVar(L"OSQUERY_EXTENSIONS").is_initialized(); + return getEnvVar("OSQUERY_EXTENSIONS").is_initialized(); } bool WatcherRunner::ok() const { @@ -551,7 +551,7 @@ void WatcherRunner::createWorker() { // Set an environment signaling to potential plugin-dependent workers to wait // for extensions to broadcast. if (watcher.hasManagedExtensions()) { - setEnvVar(L"OSQUERY_EXTENSIONS", L"true"); + setEnvVar("OSQUERY_EXTENSIONS", "true"); } // Get the complete path of the osquery process binary. diff --git a/osquery/devtools/printer.cpp b/osquery/devtools/printer.cpp index 6dbd8f99430..a4274027cfc 100644 --- a/osquery/devtools/printer.cpp +++ b/osquery/devtools/printer.cpp @@ -29,7 +29,7 @@ std::string generateToken(const std::map& lengths, std::string out = "+"; for (const auto& col : columns) { size_t size = tryTakeCopy(lengths, col).takeOr(col.size()) + 2; - if (getEnvVar(L"ENHANCE").is_initialized()) { + if (getEnvVar("ENHANCE").is_initialized()) { std::string e = "\xF0\x9F\x90\x8C"; e[2] += kOffset[1]; e[3] += kOffset[0]; @@ -58,7 +58,7 @@ std::string generateToken(const std::map& lengths, std::string generateHeader(const std::map& lengths, const std::vector& columns) { - if (getEnvVar(L"ENHANCE").is_initialized()) { + if (getEnvVar("ENHANCE").is_initialized()) { kToken = "\xF0\x9F\x91\x8D"; } std::string out = kToken; diff --git a/osquery/devtools/shell.cpp b/osquery/devtools/shell.cpp index 25dcacde123..a6d91057786 100644 --- a/osquery/devtools/shell.cpp +++ b/osquery/devtools/shell.cpp @@ -1643,7 +1643,7 @@ static void main_init(struct callback_data* data) { auto term = osquery::getEnvVar("TERM"); if (term.is_initialized() && - (*term).find("xterm-256color") != std::wstring::npos) { + (*term).find("xterm-256color") != std::string::npos) { sqlite3_snprintf( sizeof(mainPrompt), mainPrompt, "\033[38;5;147mosquery> \033[0m"); sqlite3_snprintf(sizeof(continuePrompt), From a9cefdab0423a0c7c1f915e00e971018647c3164 Mon Sep 17 00:00:00 2001 From: Ateeq Sharfuddin Date: Sun, 19 Jan 2020 19:54:46 -0500 Subject: [PATCH 04/16] reverting additional files forgotten in prev commit --- osquery/config/tests/test_utils.cpp | 3 +-- osquery/filesystem/tests/filesystem.cpp | 1 - osquery/remote/enroll/enroll.cpp | 1 - osquery/tables/system/windows/chocolatey_packages.cpp | 2 +- osquery/tables/system/windows/windows_crashes.cpp | 4 +--- osquery/utils/aws/tests/aws_util_tests.cpp | 1 - osquery/utils/system/env.h | 6 +++--- 7 files changed, 6 insertions(+), 12 deletions(-) diff --git a/osquery/config/tests/test_utils.cpp b/osquery/config/tests/test_utils.cpp index 0537cae67b6..f431ab29fac 100644 --- a/osquery/config/tests/test_utils.cpp +++ b/osquery/config/tests/test_utils.cpp @@ -10,7 +10,6 @@ #include -#include #include #include @@ -24,7 +23,7 @@ namespace { namespace fs = boost::filesystem; fs::path getConfDirPathImpl() { - std::string kEnvVarName = "TEST_CONF_FILES_DIR"; + const char* kEnvVarName = "TEST_CONF_FILES_DIR"; auto const value_opt = osquery::getEnvVar(kEnvVarName); EXPECT_TRUE(static_cast(value_opt)) << "Env var " << boost::io::quoted(kEnvVarName) << " was not found, " diff --git a/osquery/filesystem/tests/filesystem.cpp b/osquery/filesystem/tests/filesystem.cpp index 33a1b7a11b0..88919d9574d 100644 --- a/osquery/filesystem/tests/filesystem.cpp +++ b/osquery/filesystem/tests/filesystem.cpp @@ -22,7 +22,6 @@ #include #include -#include #include #include diff --git a/osquery/remote/enroll/enroll.cpp b/osquery/remote/enroll/enroll.cpp index 2a09b240492..021b6d76442 100644 --- a/osquery/remote/enroll/enroll.cpp +++ b/osquery/remote/enroll/enroll.cpp @@ -16,7 +16,6 @@ #include #include #include -#include #include namespace osquery { diff --git a/osquery/tables/system/windows/chocolatey_packages.cpp b/osquery/tables/system/windows/chocolatey_packages.cpp index 1b5b4cf2b5e..c2df1f4f153 100644 --- a/osquery/tables/system/windows/chocolatey_packages.cpp +++ b/osquery/tables/system/windows/chocolatey_packages.cpp @@ -53,7 +53,7 @@ Status genPackage(const fs::path& nuspec, Row& r) { QueryData genChocolateyPackages(QueryContext& context) { QueryData results; - auto chocoEnvInstall = getEnvVar(L"ChocolateyInstall"); + auto chocoEnvInstall = getEnvVar("ChocolateyInstall"); fs::path chocoInstallPath; if (chocoEnvInstall.is_initialized()) { diff --git a/osquery/tables/system/windows/windows_crashes.cpp b/osquery/tables/system/windows/windows_crashes.cpp index 558b3959964..a13eff8b8bf 100644 --- a/osquery/tables/system/windows/windows_crashes.cpp +++ b/osquery/tables/system/windows/windows_crashes.cpp @@ -623,9 +623,7 @@ QueryData genCrashLogs(QueryContext& context) { dumpFolderLocation = dumpFolderResults.rows()[0].at("data"); } else { auto tempDumpLoc = getEnvVar("TMP"); - dumpFolderLocation = tempDumpLoc.is_initialized() - ? *tempDumpLoc - : ""; + dumpFolderLocation = tempDumpLoc.is_initialized() ? *tempDumpLoc : ""; } if (!fs::exists(dumpFolderLocation) || diff --git a/osquery/utils/aws/tests/aws_util_tests.cpp b/osquery/utils/aws/tests/aws_util_tests.cpp index e932dec8ea1..a756a45a269 100644 --- a/osquery/utils/aws/tests/aws_util_tests.cpp +++ b/osquery/utils/aws/tests/aws_util_tests.cpp @@ -15,7 +15,6 @@ #include #include -#include #include namespace osquery { diff --git a/osquery/utils/system/env.h b/osquery/utils/system/env.h index ebb26943406..5a5e62d707b 100644 --- a/osquery/utils/system/env.h +++ b/osquery/utils/system/env.h @@ -15,16 +15,16 @@ namespace osquery { /// Set the environment variable name with value value. -bool setEnvVar(const std::wstring& name, const std::wstring& value); +bool setEnvVar(const std::string& name, const std::string& value); /// Unsets the environment variable specified by name. -bool unsetEnvVar(const std::wstring& name); +bool unsetEnvVar(const std::string& name); /** * @brief Returns the value of the specified environment variable name. * * If the environment variable does not exist, boost::none is returned. */ -boost::optional getEnvVar(const std::wstring& name); +boost::optional getEnvVar(const std::string& name); } // namespace osquery From 2a765768821a40c66c6c87121d3b74ac1f875181 Mon Sep 17 00:00:00 2001 From: Ateeq Sharfuddin Date: Sun, 19 Jan 2020 20:18:06 -0500 Subject: [PATCH 05/16] more missing reverts --- osquery/config/tests/test_utils.cpp | 2 +- osquery/tables/system/windows/scheduled_tasks.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osquery/config/tests/test_utils.cpp b/osquery/config/tests/test_utils.cpp index f431ab29fac..207a23efcbf 100644 --- a/osquery/config/tests/test_utils.cpp +++ b/osquery/config/tests/test_utils.cpp @@ -23,7 +23,7 @@ namespace { namespace fs = boost::filesystem; fs::path getConfDirPathImpl() { - const char* kEnvVarName = "TEST_CONF_FILES_DIR"; + char const* kEnvVarName = "TEST_CONF_FILES_DIR"; auto const value_opt = osquery::getEnvVar(kEnvVarName); EXPECT_TRUE(static_cast(value_opt)) << "Env var " << boost::io::quoted(kEnvVarName) << " was not found, " diff --git a/osquery/tables/system/windows/scheduled_tasks.cpp b/osquery/tables/system/windows/scheduled_tasks.cpp index e3f0d48067a..bc3604c2252 100644 --- a/osquery/tables/system/windows/scheduled_tasks.cpp +++ b/osquery/tables/system/windows/scheduled_tasks.cpp @@ -198,11 +198,11 @@ QueryData genScheduledTasks(QueryContext& context) { enumerateTasksForFolder("\\", results); // We attempt to derive the location of the tasks folder - auto sysRoot = getEnvVar(L"SystemRoot"); + auto sysRoot = getEnvVar("SystemRoot"); if (!sysRoot.is_initialized()) { return results; } - auto sysTaskPath = *sysRoot + L"\\System32\\Tasks"; + auto sysTaskPath = *sysRoot + "\\System32\\Tasks"; // Then process all tasks in subsequent folders std::vector taskPaths; From a2be033abe09d61a55af2d1acc20741b4c553740 Mon Sep 17 00:00:00 2001 From: Ateeq Sharfuddin Date: Sun, 19 Jan 2020 22:22:30 -0500 Subject: [PATCH 06/16] bad typo: length miscalculation --- osquery/utils/system/windows/env.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osquery/utils/system/windows/env.cpp b/osquery/utils/system/windows/env.cpp index dbda9c18970..b839f9beb6a 100644 --- a/osquery/utils/system/windows/env.cpp +++ b/osquery/utils/system/windows/env.cpp @@ -106,7 +106,7 @@ boost::optional getEnvVar(const std::string& name) { buf.data(), -1, narrowvalue.data(), - value_len + 1 * 4, + (value_len + 1) * 4, 0, nullptr); From 71a1b7d7c0fb9c2eadfe3532bfc9d5372d44cb6b Mon Sep 17 00:00:00 2001 From: Ateeq Sharfuddin Date: Sun, 19 Jan 2020 23:00:47 -0500 Subject: [PATCH 07/16] adding successful conversion check to retry build failure --- osquery/utils/system/windows/env.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/osquery/utils/system/windows/env.cpp b/osquery/utils/system/windows/env.cpp index b839f9beb6a..5bc07c44e38 100644 --- a/osquery/utils/system/windows/env.cpp +++ b/osquery/utils/system/windows/env.cpp @@ -101,16 +101,16 @@ boost::optional getEnvVar(const std::string& name) { } narrowvalue.assign((value_len + 1) * 4, '\0'); - WideCharToMultiByte(CP_UTF8, - 0, - buf.data(), - -1, - narrowvalue.data(), - (value_len + 1) * 4, - 0, - nullptr); - - return narrowvalue.data(); + if (0 != WideCharToMultiByte(CP_UTF8, + 0, + buf.data(), + -1, + narrowvalue.data(), + (value_len + 1) * 4, + 0, + nullptr)) { + return narrowvalue.data(); + } } return std::string(); From f3ad8f07414601609be29c02f007870dac2e8d18 Mon Sep 17 00:00:00 2001 From: Ateeq Sharfuddin Date: Sun, 26 Jan 2020 00:51:28 -0500 Subject: [PATCH 08/16] adjusting code as per review --- .../events/windows/ntfs_event_publisher.cpp | 2 +- osquery/filesystem/windows/fileops.cpp | 2 +- osquery/tables/system/windows/registry.cpp | 10 +- osquery/utils/conversions/windows/strings.cpp | 5 + osquery/utils/conversions/windows/strings.h | 7 + osquery/utils/system/CMakeLists.txt | 1 + osquery/utils/system/windows/env.cpp | 120 +++++------------- 7 files changed, 54 insertions(+), 93 deletions(-) diff --git a/osquery/events/windows/ntfs_event_publisher.cpp b/osquery/events/windows/ntfs_event_publisher.cpp index b2b9115833a..3db69c6afb0 100644 --- a/osquery/events/windows/ntfs_event_publisher.cpp +++ b/osquery/events/windows/ntfs_event_publisher.cpp @@ -220,7 +220,7 @@ Status NTFSEventPublisher::getPathFromReferenceNumber( description = L"Unknown error"; } - message << wstringToString(description.c_str()); + message << wstringToString(description); return Status::failure(message.str()); } diff --git a/osquery/filesystem/windows/fileops.cpp b/osquery/filesystem/windows/fileops.cpp index de37ea70b32..a9c9027807c 100644 --- a/osquery/filesystem/windows/fileops.cpp +++ b/osquery/filesystem/windows/fileops.cpp @@ -1424,7 +1424,7 @@ boost::optional getHomeDirectory() { std::vector profile(MAX_PATH); auto value = getEnvVar("USERPROFILE"); if (value.is_initialized()) { - return *value; + return value; } else if (SUCCEEDED(::SHGetFolderPathW( nullptr, CSIDL_PROFILE, nullptr, 0, profile.data()))) { return wstringToString(profile.data()); diff --git a/osquery/tables/system/windows/registry.cpp b/osquery/tables/system/windows/registry.cpp index 59a13519393..5ca4a3bb965 100644 --- a/osquery/tables/system/windows/registry.cpp +++ b/osquery/tables/system/windows/registry.cpp @@ -367,15 +367,15 @@ Status queryKey(const std::string& keyPath, QueryData& results) { if (bpDataBuff != nullptr) { /// REG_LINK is a Unicode string, which in Windows is wchar_t std::string regLinkStr; - // std::make_unique(cbMaxValueData); if (lpType == REG_LINK) { - regLinkStr = wstringToString((wchar_t*)bpDataBuff.get()); + regLinkStr = + wstringToString(reinterpret_cast(bpDataBuff.get())); } std::vector regBinary; std::string data; std::vector multiSzStrs; - auto p = (wchar_t*)bpDataBuff.get(); + auto p = reinterpret_cast(bpDataBuff.get()); switch (lpType) { case REG_FULL_RESOURCE_DESCRIPTOR: @@ -395,7 +395,7 @@ Status queryKey(const std::string& keyPath, QueryData& results) { r["data"] = std::to_string(_byteswap_ulong(*((int*)bpDataBuff.get()))); break; case REG_EXPAND_SZ: - r["data"] = wstringToString((wchar_t*)bpDataBuff.get()); + r["data"] = wstringToString(reinterpret_cast(bpDataBuff.get())); break; case REG_LINK: r["data"] = regLinkStr; @@ -415,7 +415,7 @@ Status queryKey(const std::string& keyPath, QueryData& results) { r["data"] = std::to_string(*((unsigned long long*)bpDataBuff.get())); break; case REG_SZ: - r["data"] = wstringToString((wchar_t*)bpDataBuff.get()); + r["data"] = wstringToString(reinterpret_cast(bpDataBuff.get())); break; default: r["data"] = ""; diff --git a/osquery/utils/conversions/windows/strings.cpp b/osquery/utils/conversions/windows/strings.cpp index 039a516a04b..285ab41410c 100644 --- a/osquery/utils/conversions/windows/strings.cpp +++ b/osquery/utils/conversions/windows/strings.cpp @@ -61,6 +61,11 @@ std::wstring stringToWstring(const std::string& src) { return utf16le_str; } +std::string wstringToString(const std::wstring& src) { + std::string utf8_str = converter.to_bytes(src); + return utf8_str; +} + std::string wstringToString(const wchar_t* src) { if (src == nullptr) { return std::string(""); diff --git a/osquery/utils/conversions/windows/strings.h b/osquery/utils/conversions/windows/strings.h index 680ff130b17..2039228fd39 100644 --- a/osquery/utils/conversions/windows/strings.h +++ b/osquery/utils/conversions/windows/strings.h @@ -27,6 +27,13 @@ std::wstring stringToWstring(const std::string& src); * * @returns A narrow string, constructed from a wide string */ +std::string wstringToString(const std::wstring& src); + +/** + * @brief Windows helper function for converting wide C-strings to narrow + * + * @returns A narrow string, constructed from a wide C-string + */ std::string wstringToString(const wchar_t* src); /** diff --git a/osquery/utils/system/CMakeLists.txt b/osquery/utils/system/CMakeLists.txt index e4d62db3a3c..06879cf2196 100644 --- a/osquery/utils/system/CMakeLists.txt +++ b/osquery/utils/system/CMakeLists.txt @@ -42,6 +42,7 @@ function(generateOsqueryUtilsSystemEnv) target_link_libraries(osquery_utils_system_env PUBLIC osquery_cxx_settings + osquery_utils_conversions thirdparty_boost ) diff --git a/osquery/utils/system/windows/env.cpp b/osquery/utils/system/windows/env.cpp index 5bc07c44e38..2a163ff7279 100644 --- a/osquery/utils/system/windows/env.cpp +++ b/osquery/utils/system/windows/env.cpp @@ -6,6 +6,7 @@ * the LICENSE file found in the root directory of this source tree. */ +#include #include #include @@ -18,102 +19,49 @@ namespace osquery { bool setEnvVar(const std::string& name, const std::string& value) { - bool status = false; - std::vector widename; - widename.assign((name.length() + 1) * 2, '\0'); - - std::vector widevalue; - widevalue.assign((value.length() + 1) * 2, '\0'); - - if (0 != MultiByteToWideChar(CP_UTF8, - 0, - name.c_str(), - -1, - widename.data(), - (name.length() + 1) * 2)) { - if (0 != MultiByteToWideChar(CP_UTF8, - 0, - value.c_str(), - -1, - widevalue.data(), - (value.length() + 1) * 2)) { - status = (::SetEnvironmentVariableW(widename.data(), widevalue.data()) == - TRUE); - } - } + std::wstring widename = stringToWstring(name); + std::wstring widevalue = stringToWstring(value); - return status; + return ::SetEnvironmentVariableW(widename.c_str(), widevalue.c_str()) == TRUE; } bool unsetEnvVar(const std::string& name) { - bool status = false; - std::vector widename; - widename.assign((name.length() + 1) * 2, '\0'); - - if (0 != MultiByteToWideChar(CP_UTF8, - 0, - name.c_str(), - -1, - widename.data(), - (name.length() + 1) * 2)) { - status = (::SetEnvironmentVariableW(widename.data(), nullptr) == TRUE); - } - - return status; + std::wstring widename = stringToWstring(name); + return ::SetEnvironmentVariableW(widename.c_str(), nullptr) == TRUE; } boost::optional getEnvVar(const std::string& name) { const auto kInitialBufferSize = 1024; std::vector buf; - buf.assign(kInitialBufferSize, '\0'); - - std::vector widename; - widename.assign((name.length() + 1) * 2, '\0'); - - std::vector narrowvalue; - - if (0 != MultiByteToWideChar(CP_UTF8, - 0, - name.c_str(), - -1, - widename.data(), - (name.length() + 1) * 2)) { - auto value_len = ::GetEnvironmentVariableW( - widename.data(), buf.data(), kInitialBufferSize); - if (value_len == 0) { - return boost::none; - } - - // It is always possible that between the first GetEnvironmentVariableA call - // and this one, a change was made to our target environment variable that - // altered the size. Currently, we ignore this scenario and fail if the - // returned size is greater than what we expect. - if (value_len > kInitialBufferSize) { - buf.assign(value_len, '\0'); - value_len = - ::GetEnvironmentVariableW(widename.data(), buf.data(), value_len); - if (value_len == 0 || value_len > buf.size()) { - // The size returned is greater than the size we expected. Currently, we - // will not deal with this scenario and just return as if an error has - // occurred. - return boost::none; - } - } - - narrowvalue.assign((value_len + 1) * 4, '\0'); - if (0 != WideCharToMultiByte(CP_UTF8, - 0, - buf.data(), - -1, - narrowvalue.data(), - (value_len + 1) * 4, - 0, - nullptr)) { - return narrowvalue.data(); - } + buf.assign(kInitialBufferSize, L'\0'); + + std::wstring widename = stringToWstring(name); + + auto value_len = ::GetEnvironmentVariableW( + widename.c_str(), buf.data(), kInitialBufferSize); + if (value_len == 0) { + return boost::none; } - return std::string(); -} + // It is always possible that between the first GetEnvironmentVariableA call + // and this one, a change was made to our target environment variable that + // altered the size. Currently, we ignore this scenario and fail if the + // returned size is greater than what we expect. + if (value_len > kInitialBufferSize) { + buf.assign(value_len, '\0'); + value_len = + ::GetEnvironmentVariableW(widename.c_str(), buf.data(), value_len); + } + + if (value_len == 0 || value_len > buf.size()) { + // The size returned is greater than the size we expected. Currently, we + // will not deal with this scenario and just return as if an error has + // occurred. + return boost::none; + } + + return wstringToString(buf.data()); + +} // namespace osquery } // namespace osquery From 0718ee3f993610ac7adf6447fdf06c767e05d9f1 Mon Sep 17 00:00:00 2001 From: Ateeq Sharfuddin Date: Sun, 26 Jan 2020 00:55:10 -0500 Subject: [PATCH 09/16] removing TODO comment and created ticket --- osquery/filesystem/windows/fileops.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/osquery/filesystem/windows/fileops.cpp b/osquery/filesystem/windows/fileops.cpp index a9c9027807c..906f29f5004 100644 --- a/osquery/filesystem/windows/fileops.cpp +++ b/osquery/filesystem/windows/fileops.cpp @@ -1488,10 +1488,6 @@ static std::string normalizeDirPath(const fs::path& path) { } // NTFS is case insensitive, to normalize, make everything uppercase - // TODO: the above comment is incorrect: - // 1. NTFS is actually case-sensitive. - // 2. OS might have case-sensitivity turned on, which will - // produce false-positives here. ::CharUpperW(final_path.data()); boost::system::error_code ec; From 8e1ae05bc52bddf319580a8253e1cda825fe3680 Mon Sep 17 00:00:00 2001 From: Ateeq Sharfuddin Date: Sun, 26 Jan 2020 01:36:08 -0500 Subject: [PATCH 10/16] check style --- osquery/tables/system/windows/registry.cpp | 6 ++++-- osquery/utils/conversions/windows/strings.h | 8 ++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/osquery/tables/system/windows/registry.cpp b/osquery/tables/system/windows/registry.cpp index 5ca4a3bb965..f4cc4bf2d65 100644 --- a/osquery/tables/system/windows/registry.cpp +++ b/osquery/tables/system/windows/registry.cpp @@ -395,7 +395,8 @@ Status queryKey(const std::string& keyPath, QueryData& results) { r["data"] = std::to_string(_byteswap_ulong(*((int*)bpDataBuff.get()))); break; case REG_EXPAND_SZ: - r["data"] = wstringToString(reinterpret_cast(bpDataBuff.get())); + r["data"] = + wstringToString(reinterpret_cast(bpDataBuff.get())); break; case REG_LINK: r["data"] = regLinkStr; @@ -415,7 +416,8 @@ Status queryKey(const std::string& keyPath, QueryData& results) { r["data"] = std::to_string(*((unsigned long long*)bpDataBuff.get())); break; case REG_SZ: - r["data"] = wstringToString(reinterpret_cast(bpDataBuff.get())); + r["data"] = + wstringToString(reinterpret_cast(bpDataBuff.get())); break; default: r["data"] = ""; diff --git a/osquery/utils/conversions/windows/strings.h b/osquery/utils/conversions/windows/strings.h index 2039228fd39..bd6bab38f62 100644 --- a/osquery/utils/conversions/windows/strings.h +++ b/osquery/utils/conversions/windows/strings.h @@ -30,10 +30,10 @@ std::wstring stringToWstring(const std::string& src); std::string wstringToString(const std::wstring& src); /** - * @brief Windows helper function for converting wide C-strings to narrow - * - * @returns A narrow string, constructed from a wide C-string - */ +* @brief Windows helper function for converting wide C-strings to narrow +* +* @returns A narrow string, constructed from a wide C-string +*/ std::string wstringToString(const wchar_t* src); /** From 5c67f6e9d5753cfa6c6fcf09cf52d6005c0f8d37 Mon Sep 17 00:00:00 2001 From: Ateeq Sharfuddin Date: Sun, 26 Jan 2020 09:04:36 -0500 Subject: [PATCH 11/16] check style --- osquery/utils/conversions/windows/strings.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/osquery/utils/conversions/windows/strings.h b/osquery/utils/conversions/windows/strings.h index bd6bab38f62..b7b480ef6ea 100644 --- a/osquery/utils/conversions/windows/strings.h +++ b/osquery/utils/conversions/windows/strings.h @@ -30,10 +30,10 @@ std::wstring stringToWstring(const std::string& src); std::string wstringToString(const std::wstring& src); /** -* @brief Windows helper function for converting wide C-strings to narrow -* -* @returns A narrow string, constructed from a wide C-string -*/ + * @brief Windows helper function for converting wide C-strings to narrow + * + * @returns A narrow string, constructed from a wide C-string + */ std::string wstringToString(const wchar_t* src); /** From dc71c56aba11fb195f06f966a7d7d7567c354170 Mon Sep 17 00:00:00 2001 From: Ateeq Sharfuddin Date: Sun, 26 Jan 2020 09:35:03 -0500 Subject: [PATCH 12/16] Updating utils/system/BUCK to add utils/conversions dependencies --- osquery/utils/system/BUCK | 1 + 1 file changed, 1 insertion(+) diff --git a/osquery/utils/system/BUCK b/osquery/utils/system/BUCK index 1b7195e2a4b..0f0448be858 100644 --- a/osquery/utils/system/BUCK +++ b/osquery/utils/system/BUCK @@ -217,6 +217,7 @@ osquery_cxx_library( ], visibility = ["PUBLIC"], deps = [ + osquery_target("osquery/utils/conversions:conversions"), osquery_target("osquery/utils/info:info"), osquery_tp_target("boost"), osquery_tp_target("googletest", "gtest_headers"), From 2b4149ead4ab541f5d0996d5ba1fa18bb941a2a2 Mon Sep 17 00:00:00 2001 From: Ateeq Sharfuddin Date: Sun, 26 Jan 2020 19:44:46 -0500 Subject: [PATCH 13/16] updates per PR comments and buck build failure --- osquery/utils/system/BUCK | 2 +- osquery/utils/system/windows/env.cpp | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/osquery/utils/system/BUCK b/osquery/utils/system/BUCK index 0f0448be858..575768edebf 100644 --- a/osquery/utils/system/BUCK +++ b/osquery/utils/system/BUCK @@ -31,6 +31,7 @@ osquery_cxx_library( ], visibility = ["PUBLIC"], deps = [ + osquery_target("osquery/utils/conversions:conversions"), osquery_tp_target("boost"), ], ) @@ -217,7 +218,6 @@ osquery_cxx_library( ], visibility = ["PUBLIC"], deps = [ - osquery_target("osquery/utils/conversions:conversions"), osquery_target("osquery/utils/info:info"), osquery_tp_target("boost"), osquery_tp_target("googletest", "gtest_headers"), diff --git a/osquery/utils/system/windows/env.cpp b/osquery/utils/system/windows/env.cpp index 2a163ff7279..f43a04a0403 100644 --- a/osquery/utils/system/windows/env.cpp +++ b/osquery/utils/system/windows/env.cpp @@ -19,14 +19,14 @@ namespace osquery { bool setEnvVar(const std::string& name, const std::string& value) { - std::wstring widename = stringToWstring(name); - std::wstring widevalue = stringToWstring(value); + const std::wstring widename = stringToWstring(name); + const std::wstring widevalue = stringToWstring(value); return ::SetEnvironmentVariableW(widename.c_str(), widevalue.c_str()) == TRUE; } bool unsetEnvVar(const std::string& name) { - std::wstring widename = stringToWstring(name); + const std::wstring widename = stringToWstring(name); return ::SetEnvironmentVariableW(widename.c_str(), nullptr) == TRUE; } @@ -35,7 +35,7 @@ boost::optional getEnvVar(const std::string& name) { std::vector buf; buf.assign(kInitialBufferSize, L'\0'); - std::wstring widename = stringToWstring(name); + const std::wstring widename = stringToWstring(name); auto value_len = ::GetEnvironmentVariableW( widename.c_str(), buf.data(), kInitialBufferSize); From f60a049e5dacf5240ae3ff3736b1f944124a16cc Mon Sep 17 00:00:00 2001 From: Ateeq Sharfuddin Date: Wed, 5 Feb 2020 20:33:39 -0500 Subject: [PATCH 14/16] accomodate updated function prototype --- osquery/utils/system/windows/env.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/osquery/utils/system/windows/env.cpp b/osquery/utils/system/windows/env.cpp index dba8fe88fec..ebd0de3ec99 100644 --- a/osquery/utils/system/windows/env.cpp +++ b/osquery/utils/system/windows/env.cpp @@ -88,11 +88,12 @@ boost::optional expandEnvString(const std::string& input) { auto len = ::ExpandEnvironmentStrings(input.c_str(), buf.data(), kInitialBufferSize); if (len == 0) { - std::string description; + std::wstring description; if (!getWindowsErrorDescription(description, ::GetLastError())) { - description = "Unknown error"; + description = L"Unknown error"; } - VLOG(1) << "Failed to expand environment string: " << description; + VLOG(1) << "Failed to expand environment string: " + << wstringToString(description); return boost::none; } @@ -103,11 +104,12 @@ boost::optional expandEnvString(const std::string& input) { } if (len == 0) { - std::string description; + std::wstring description; if (!getWindowsErrorDescription(description, ::GetLastError())) { - description = "Unknown error"; + description = L"Unknown error"; } - VLOG(1) << "Failed to expand environment string: " << description; + VLOG(1) << "Failed to expand environment string: " + << wstringToString(description); return boost::none; } From 50933570f8493a09da307e369273d36dc02a1790 Mon Sep 17 00:00:00 2001 From: Ateeq Sharfuddin Date: Mon, 10 Feb 2020 07:48:49 -0500 Subject: [PATCH 15/16] remove stray copy-paste --- osquery/utils/system/windows/env.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osquery/utils/system/windows/env.cpp b/osquery/utils/system/windows/env.cpp index ebd0de3ec99..d686e6f235c 100644 --- a/osquery/utils/system/windows/env.cpp +++ b/osquery/utils/system/windows/env.cpp @@ -73,7 +73,7 @@ boost::optional getEnvVar(const std::string& name) { return wstringToString(buf.data()); -} // namespace osquery +} boost::optional expandEnvString(const std::string& input) { std::vector buf; From 9a5dec8acd2f75e522717034643afcb4cd2a902f Mon Sep 17 00:00:00 2001 From: Ateeq Sharfuddin Date: Mon, 10 Feb 2020 07:55:59 -0500 Subject: [PATCH 16/16] formatting: remove extra line --- osquery/utils/system/windows/env.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/osquery/utils/system/windows/env.cpp b/osquery/utils/system/windows/env.cpp index d686e6f235c..a044e66e8be 100644 --- a/osquery/utils/system/windows/env.cpp +++ b/osquery/utils/system/windows/env.cpp @@ -72,7 +72,6 @@ boost::optional getEnvVar(const std::string& name) { } return wstringToString(buf.data()); - } boost::optional expandEnvString(const std::string& input) {