From 08dd45f5044522d4390e994c9cda18360244dba1 Mon Sep 17 00:00:00 2001
From: Louis Moureaux <m_louis30@yahoo.com>
Date: Mon, 1 Jan 2024 23:37:46 +0100
Subject: [PATCH 1/2] Use QString in is_safe_filename

---
 utility/shared.cpp | 27 +++++----------------------
 utility/shared.h   |  2 +-
 2 files changed, 6 insertions(+), 23 deletions(-)

diff --git a/utility/shared.cpp b/utility/shared.cpp
index 943d413a86..e32bdb25e2 100644
--- a/utility/shared.cpp
+++ b/utility/shared.cpp
@@ -33,6 +33,7 @@
 #include <QCoreApplication>
 #include <QDateTime>
 #include <QDir>
+#include <QRegularExpression>
 #include <QStandardPaths>
 #include <QString>
 #include <QtGlobal>
@@ -208,29 +209,11 @@ static bool is_ascii(char ch)
    Check if the name is safe security-wise.  This is intended to be used to
    make sure an untrusted filename is safe to be used.
  */
-bool is_safe_filename(const char *name)
+bool is_safe_filename(const QString &name)
 {
-  int i = 0;
-
-  // must not be nullptr or empty
-  if (!name || *name == '\0') {
-    return false;
-  }
-
-  for (; '\0' != name[i]; i++) {
-    if (nullptr == strchr(".@", name[i])
-        && nullptr == strchr(base64url, name[i])) {
-      return false;
-    }
-  }
-
-  // we don't allow the filename to ascend directories
-  if (strstr(name, PARENT_DIR_OPERATOR)) {
-    return false;
-  }
-
-  // Otherwise, it is okay...
-  return true;
+  const QRegularExpression regex(QLatin1String("^[\\w_\\-.@]+$"));
+  return regex.match(name).hasMatch()
+         && !name.contains(QLatin1String(PARENT_DIR_OPERATOR));
 }
 
 /**
diff --git a/utility/shared.h b/utility/shared.h
index 998cb317f4..bf1f3045f4 100644
--- a/utility/shared.h
+++ b/utility/shared.h
@@ -107,7 +107,7 @@ const char *int_to_text(unsigned int number);
 
 bool is_ascii_name(const char *name);
 bool is_base64url(const char *s);
-bool is_safe_filename(const char *name);
+bool is_safe_filename(const QString &name);
 void randomize_base64url_string(char *s, size_t n);
 
 char *skip_leading_spaces(char *s);

From 6591fead1e759fa393398151f2954280693673ae Mon Sep 17 00:00:00 2001
From: Louis Moureaux <m_louis30@yahoo.com>
Date: Mon, 1 Jan 2024 23:38:22 +0100
Subject: [PATCH 2/2] Add test for is_safe_filename

---
 utility/CMakeLists.txt       |  5 +++++
 utility/tests/CMakeLists.txt |  5 +++++
 utility/tests/test_paths.cpp | 27 +++++++++++++++++++++++++++
 3 files changed, 37 insertions(+)
 create mode 100644 utility/tests/CMakeLists.txt
 create mode 100644 utility/tests/test_paths.cpp

diff --git a/utility/CMakeLists.txt b/utility/CMakeLists.txt
index e7fbfe1706..8c4b29e57a 100644
--- a/utility/CMakeLists.txt
+++ b/utility/CMakeLists.txt
@@ -82,3 +82,8 @@ if(FREECIV_ENABLE_NLS)
   target_include_directories(utility PUBLIC ${Intl_INCLUDE_DIRS})
   target_link_libraries(utility PUBLIC ${Intl_LIBRARIES})
 endif()
+
+# Tests
+if (BUILD_TESTING)
+  add_subdirectory(tests)
+endif()
diff --git a/utility/tests/CMakeLists.txt b/utility/tests/CMakeLists.txt
new file mode 100644
index 0000000000..2a25ad5537
--- /dev/null
+++ b/utility/tests/CMakeLists.txt
@@ -0,0 +1,5 @@
+set(CMAKE_AUTOMOC ON)
+
+add_executable(test_utility_paths test_paths.cpp)
+target_link_libraries(test_utility_paths PRIVATE Qt5::Test utility)
+add_test(NAME test_utility_paths COMMAND test_utility_paths)
diff --git a/utility/tests/test_paths.cpp b/utility/tests/test_paths.cpp
new file mode 100644
index 0000000000..8c10d7c21d
--- /dev/null
+++ b/utility/tests/test_paths.cpp
@@ -0,0 +1,27 @@
+#include "shared.h"
+
+#include <QtTest>
+
+/**
+ * Tests functions acting on paths
+ */
+class test_paths : public QObject {
+  Q_OBJECT
+
+private slots:
+  void is_safe_filename();
+};
+
+/**
+ * Tests \ref ::is_safe_filename
+ */
+void test_paths::is_safe_filename()
+{
+  QCOMPARE(::is_safe_filename(QLatin1String("")), false);
+  QCOMPARE(::is_safe_filename(QLatin1String("abcABC_-._")), true);
+  QCOMPARE(::is_safe_filename(QLatin1String("a/b")), false);
+  QCOMPARE(::is_safe_filename(QLatin1String("..")), false);
+}
+
+QTEST_MAIN(test_paths)
+#include "test_paths.moc"