Fix CodeQL violations of type 'cpp/path-injection in lib/pal (#1332)

* Fix CodeQL violations of type 'cpp/path-injection in lib/pal

* fix build error and test

* fix tests

* fix tests

* use expect_throw for runtime_error

* fix build error

* address comments

Author: abuhsayem

lib/pal/PAL.cpp

@@ -47,6 +47,7 @@
 #include <Objbase.h>
 #pragma comment(lib, "Ole32.Lib")   /* CoCreateGuid */
 #include <oacr.h>
+#include <windows.h>
 #endif
 
 #ifdef ANDROID
@@ -113,8 +114,27 @@ namespace PAL_NS_BEGIN {
                 return result;
             }
 
+            // Check if the path exists
+#if defined(_WIN32) || defined(_WIN64)
+            DWORD fileAttr = GetFileAttributesA(traceFolderPath.c_str());
+            bool pathExists = (fileAttr != INVALID_FILE_ATTRIBUTES && (fileAttr & FILE_ATTRIBUTE_DIRECTORY));
+#else
+            bool pathExists = (access(traceFolderPath.c_str(), F_OK) != -1);
+#endif
+            // Check if the path contains ".."
+            bool containsParentDirectory = (traceFolderPath.find("..") != std::string::npos);
+
+            if (!pathExists || containsParentDirectory)
+            {
+                return false;
+            }
+
             debugLogMutex.lock();
             debugLogPath = traceFolderPath;
+            if (debugLogPath.back() != '/' && debugLogPath.back() != '\\')
+            {
+                debugLogPath += "/";
+            }
             debugLogPath += "mat-debug-";
             debugLogPath += std::to_string(MAT::GetCurrentProcessId());
             debugLogPath += ".log";
@@ -131,6 +151,16 @@ namespace PAL_NS_BEGIN {
             return result;
         }
 
+        const std::unique_ptr<std::fstream>& getDebugLogStream() noexcept
+        {
+            return debugLogStream;
+        }
+
+        const std::string& getDebugLogPath() noexcept
+        {
+            return debugLogPath;
+        }
+
         void log_done()
         {
             debugLogMutex.lock();

lib/pal/PAL.hpp

@@ -239,6 +239,16 @@ namespace PAL_NS_BEGIN
         return GetPAL().RegisterIkeyWithWindowsTelemetry(ikeyin, storageSize, uploadQuotaSize);
     }
 
+#ifdef HAVE_MAT_LOGGING
+    namespace detail
+    {
+        bool log_init(bool isTraceEnabled, const std::string& traceFolderPath);
+        const std::unique_ptr<std::fstream>& getDebugLogStream() noexcept;
+        const std::string& getDebugLogPath() noexcept;
+        void log_done();
+    }
+#endif
+
 } PAL_NS_END
 
 #endif

tests/unittests/PalTests.cpp

@@ -7,6 +7,21 @@
 #include "pal/PseudoRandomGenerator.hpp"
 #include "Version.hpp"
 
+#ifdef HAVE_MAT_LOGGING
+#include "pal/PAL.hpp"
+#include <gtest/gtest.h>
+#include <fstream>
+#include <memory>
+#include <string>
+#include <cstdio>
+#if defined(_WIN32) || defined(_WIN64)
+#include <windows.h>
+#else
+#include <unistd.h>
+#endif
+using namespace PAL::detail;
+#endif
+
 using namespace testing;
 
 class PalTests : public Test {};
@@ -127,3 +142,88 @@ TEST_F(PalTests, SdkVersion)
 
     EXPECT_THAT(PAL::getSdkVersion(), Eq(v));
 }
+
+#ifdef HAVE_MAT_LOGGING
+class LogInitTest : public Test
+{
+    protected:
+        const std::string validPath = "valid/path/";
+
+        void SetUp() override
+        {
+            // Create the valid path directory and any intermediate directories
+    #if defined(_WIN32) || defined(_WIN64)
+            CreateDirectoryA("valid", NULL);
+            CreateDirectoryA(validPath.c_str(), NULL);
+    #else
+            mkdir("valid", 0777);
+            mkdir(validPath.c_str(), 0777);
+    #endif
+        }
+
+        void TearDown() override
+        {
+            PAL::detail::log_done();
+            if (!PAL::detail::getDebugLogPath().empty())
+            {
+                std::remove(PAL::detail::getDebugLogPath().c_str());
+            }
+
+            // Remove the valid path directory
+    #if defined(_WIN32) || defined(_WIN64)
+            RemoveDirectoryA(validPath.c_str());
+            RemoveDirectoryA("valid");
+    #else
+            rmdir(validPath.c_str());
+            rmdir("valid");
+    #endif
+        }
+};
+
+TEST_F(LogInitTest, LogInitDisabled)
+{
+    EXPECT_FALSE(log_init(false, validPath));
+}
+
+TEST_F(LogInitTest, LogInitValidPath)
+{
+    EXPECT_TRUE(PAL::detail::log_init(true, validPath));
+    EXPECT_TRUE(PAL::detail::getDebugLogStream()->is_open());
+}
+
+TEST_F(LogInitTest, LogInitParentDirectoryInvalidPath)
+{
+    EXPECT_FALSE(PAL::detail::log_init(true, "invalid/../path/"));
+}
+
+TEST_F(LogInitTest, LogInitPathDoesNotExist)
+{
+    EXPECT_FALSE(PAL::detail::log_init(true, "nonexistent/path/"));
+}
+
+TEST_F(LogInitTest, LogInitAlreadyInitialized)
+{
+    EXPECT_TRUE(PAL::detail::log_init(true, validPath));
+    EXPECT_TRUE(PAL::detail::getDebugLogStream()->is_open());
+    EXPECT_TRUE(PAL::detail::log_init(true, validPath)); // Should return true as it's already initialized
+}
+
+TEST_F(LogInitTest, LogInitPathWithoutTrailingSlash)
+{
+    std::string pathWithoutSlash = "valid/path";
+    EXPECT_TRUE(PAL::detail::log_init(true, pathWithoutSlash));
+    EXPECT_TRUE(PAL::detail::getDebugLogStream()->is_open());
+}
+
+TEST_F(LogInitTest, LogInitPathWithoutTrailingBackslash)
+{
+#if defined(_WIN32) || defined(_WIN64)
+    std::string pathWithoutBackslash = "valid\\path";
+#else
+    std::string pathWithoutBackslash = "valid//path";
+#endif
+    EXPECT_TRUE(PAL::detail::log_init(true, pathWithoutBackslash));
+    EXPECT_TRUE(PAL::detail::getDebugLogStream()->is_open());
+}
+
+#endif