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/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); 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"