fix(common): resolve a trailing ".." in NSSystemPath::ShortenPath

CMakeLists.txt

@@ -39,6 +39,7 @@ else()
         add_subdirectory( "${CORE_ROOT_DIR}/OfficeUtils/tests" officeutils_test )
         add_subdirectory( "${CORE_ROOT_DIR}/OdfFile/Reader/Converter/SMCustomShape2OOXML/TestSMCustomShape" starmath_smcustomshape_test )
         add_subdirectory( "${CORE_ROOT_DIR}/OdfFile/Test/test_odf" test_odf )
+        add_subdirectory( "${CORE_ROOT_DIR}/DesktopEditor/common/test" path_test )
     endif()
 
 endif()

DesktopEditor/common/Path.cpp

@@ -213,7 +213,7 @@ namespace NSSystemPath
 
 		if (L".." == wsToken)
 		{
-			if (!arStack.empty() && L".." == arStack.top())
+			if (!arStack.empty() && L".." != arStack.top())
 				arStack.pop();
 			else
 				arStack.push(wsToken);
@@ -237,6 +237,12 @@ namespace NSSystemPath
 			arStack.pop();
 		}
 
+		// wsNewPath stays empty when the loop above breaks on the first entry
+		// (bRemoveExternalPath with a leading ".."), and pop_back() on an empty
+		// string is undefined behaviour.
+		if (wsNewPath.empty())
+			return std::wstring();
+
 		wsNewPath.pop_back();
 
 		#if !defined(_WIN32) && !defined(_WIN64)

DesktopEditor/common/test/CMakeLists.txt

@@ -0,0 +1,19 @@
+cmake_minimum_required(VERSION 3.10)
+
+project(path_test)
+
+set(CORE_ROOT_DIR "${CMAKE_CURRENT_LIST_DIR}/../../..")
+
+include(${CORE_ROOT_DIR}/common.cmake)
+
+# NSSystemPath is part of kernel.
+if(NOT TARGET kernel)
+    add_subdirectory(${CORE_ROOT_DIR}/Common kernel)
+endif()
+
+add_core_gtest(
+    NAME    path_test
+    SOURCES path.cpp
+    LIBS    kernel
+    GTEST_MAIN
+)

DesktopEditor/common/test/path.cpp

@@ -0,0 +1,77 @@
+#include "gtest/gtest.h"
+
+#include "../Path.h"
+
+// ShortenPath resolves "." and ".." lexically, without touching the filesystem.
+// Its result is used as a containment check by every one of its callers: the
+// ZipSlip guard in OfficeUtils/src/ZipUtilsCP.cpp, the "../" rejection in
+// EpubFile/src/CEpubFile.cpp, and several starts_with(root) checks in HtmlFile2,
+// OFDFile and the SVG image loader. It therefore decides whether a path taken
+// from a document is accepted, which is why it is worth pinning precisely.
+
+TEST(ShortenPath, keeps_a_plain_relative_path)
+{
+	EXPECT_EQ(L"a/b/c", NSSystemPath::ShortenPath(L"a/b/c"));
+}
+
+TEST(ShortenPath, resolves_a_parent_reference_in_the_middle)
+{
+	EXPECT_EQ(L"b", NSSystemPath::ShortenPath(L"a/../b"));
+}
+
+// --- defects reproduced below ---
+
+TEST(ShortenPath, resolves_a_trailing_parent_reference)
+{
+	EXPECT_EQ(L"", NSSystemPath::ShortenPath(L"a/.."));
+}
+
+TEST(ShortenPath, keeps_consecutive_parent_references)
+{
+	EXPECT_EQ(L"../..", NSSystemPath::ShortenPath(L"../.."));
+}
+
+TEST(ShortenPath, does_not_crash_on_bare_parent_when_removing_external_path)
+{
+	EXPECT_EQ(L"", NSSystemPath::ShortenPath(L"..", true));
+}
+
+// --- guards: behaviour that must not change (these pass before the fix too) ---
+
+TEST(ShortenPath, keeps_a_leading_parent_chain)
+{
+	EXPECT_EQ(L"../../etc/passwd", NSSystemPath::ShortenPath(L"../../etc/passwd"));
+}
+
+TEST(ShortenPath, keeps_an_absolute_path)
+{
+	EXPECT_EQ(L"/tmp/evil.txt", NSSystemPath::ShortenPath(L"/tmp/evil.txt"));
+}
+
+TEST(ShortenPath, returns_empty_for_empty_input)
+{
+	EXPECT_EQ(L"", NSSystemPath::ShortenPath(L""));
+}
+
+TEST(ShortenPath, drops_current_directory_segments)
+{
+	EXPECT_EQ(L"a/b", NSSystemPath::ShortenPath(L"./a/./b"));
+}
+
+// Archive entries and HTML sources routinely use backslashes; the callers rely on
+// them being treated as separators rather than as part of a name.
+TEST(ShortenPath, treats_a_backslash_as_a_separator)
+{
+	EXPECT_EQ(L"b", NSSystemPath::ShortenPath(L"a\\..\\b"));
+}
+
+TEST(ShortenPath, collapses_repeated_and_trailing_separators)
+{
+	EXPECT_EQ(L"a/b", NSSystemPath::ShortenPath(L"a//b"));
+	EXPECT_EQ(L"a/b", NSSystemPath::ShortenPath(L"a/b/"));
+}
+
+TEST(ShortenPath, drops_the_external_prefix_when_asked)
+{
+	EXPECT_EQ(L"a", NSSystemPath::ShortenPath(L"../a", true));
+}