From cc48a08f46736f69053a44c4cd2b203d48c84a5c Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Thu, 18 Sep 2025 12:37:33 -0500 Subject: [PATCH] App: Sanitize all paths for null characters (#23821) * App: Sanitize all paths for null characters * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Apply suggestions from code review --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Kacper Donat --- src/App/ApplicationDirectories.cpp | 23 ++++++++++++++++------- src/App/ApplicationDirectories.h | 6 ++++++ tests/src/App/ApplicationDirectories.cpp | 24 ++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/App/ApplicationDirectories.cpp b/src/App/ApplicationDirectories.cpp index 76e5d44e3d..916cbb8a98 100644 --- a/src/App/ApplicationDirectories.cpp +++ b/src/App/ApplicationDirectories.cpp @@ -273,11 +273,19 @@ bool ApplicationDirectories::startSafeMode(std::map& mC return false; } +std::filesystem::path ApplicationDirectories::sanitizePath(const std::string& pathAsString) +{ + size_t positionOfFirstNull = pathAsString.find('\0'); + if (positionOfFirstNull != std::string::npos) { + return {pathAsString.substr(0, positionOfFirstNull)}; + } + return {pathAsString}; +} void ApplicationDirectories::configureResourceDirectory(const std::map& mConfig) { #ifdef RESOURCEDIR - // #6892: Conda may inject null characters => remove them using c_str() - fs::path path {std::string(RESOURCEDIR).c_str()}; + // #6892: Conda may inject null characters + fs::path path = sanitizePath(RESOURCEDIR); if (path.is_absolute()) { _resource = path; } else { @@ -290,8 +298,8 @@ void ApplicationDirectories::configureResourceDirectory(const std::map& mConfig) { #ifdef LIBRARYDIR - // #6892: Conda may inject null characters => remove them using c_str() - fs::path path {std::string(LIBRARYDIR).c_str()}; + // #6892: Conda may inject null characters + fs::path path = sanitizePath(LIBRARYDIR); if (path.is_absolute()) { _library = path; } else { @@ -306,8 +314,8 @@ void ApplicationDirectories::configureLibraryDirectory(const std::map& mConfig) { #ifdef DOCDIR - // #6892: Conda may inject null characters => remove them using c_str() - fs::path path {std::string(DOCDIR).c_str()}; + // #6892: Conda may inject null characters + fs::path path = sanitizePath(DOCDIR); if (path.is_absolute()) { _help = path; } else { @@ -332,7 +340,8 @@ fs::path ApplicationDirectories::getUserHome() if (!result || error != 0) { throw Base::RuntimeError("Getting HOME path from system failed!"); } - path = Base::FileInfo::stringToPath(result->pw_dir); + std::string sanitizedPath = sanitizePath(pwd.pw_dir); + path = Base::FileInfo::stringToPath(sanitizedPath); #else path = Base::FileInfo::stringToPath(QStandardPaths::writableLocation(QStandardPaths::HomeLocation).toStdString()); #endif diff --git a/src/App/ApplicationDirectories.h b/src/App/ApplicationDirectories.h index 0b2dcc6bc2..de24b7aa34 100644 --- a/src/App/ApplicationDirectories.h +++ b/src/App/ApplicationDirectories.h @@ -233,6 +233,12 @@ namespace App { /// \return The version tuple. static std::tuple extractVersionFromConfigMap(const std::map &config); + /// A utility method to remove any stray null characters from a path (Conda sometimes + /// injects these for unknown reasons -- see #6892 in the bug tracker). + /// \param pathAsString The std::string path to sanitize + /// \returns A path with any stray nulls removed + static std::filesystem::path sanitizePath(const std::string& pathAsString); + private: std::tuple _currentVersion; std::filesystem::path _home; diff --git a/tests/src/App/ApplicationDirectories.cpp b/tests/src/App/ApplicationDirectories.cpp index 9c804e7698..b23cc9c4ca 100644 --- a/tests/src/App/ApplicationDirectories.cpp +++ b/tests/src/App/ApplicationDirectories.cpp @@ -52,6 +52,11 @@ public: { return extractVersionFromConfigMap(config); } + + static std::filesystem::path wrapSanitizePath(const std::string& pathAsString) + { + return sanitizePath(pathAsString); + } }; class ApplicationDirectoriesTest: public ::testing::Test @@ -747,6 +752,25 @@ TEST_F(ApplicationDirectoriesTest, extractVersionNegativeNumbersPassThrough) EXPECT_EQ(min, -7); } + +TEST_F(ApplicationDirectoriesTest, sanitizeRemovesNullCharacterAtEnd) +{ + std::string input = std::string("valid_path") + '\0' + "junk_after"; + std::filesystem::path result = ApplicationDirectoriesTestClass::wrapSanitizePath(input); + + EXPECT_EQ(result.string(), "valid_path"); + EXPECT_EQ(result.string().find('\0'), std::string::npos); +} + +TEST_F(ApplicationDirectoriesTest, sanitizeReturnsUnchangedIfNoNullCharacter) +{ + std::string input = "clean_path/without_nulls"; + std::filesystem::path result = ApplicationDirectoriesTestClass::wrapSanitizePath(input); + + EXPECT_EQ(result.string(), input); + EXPECT_EQ(result.string().find('\0'), std::string::npos); +} + /* NOLINTEND( readability-magic-numbers, cppcoreguidelines-avoid-magic-numbers,