Validating filenames before opening files for read / write (OpenChemi…

…stry#1367) * First step towards validating filenames * Add validation to the file format dialog Signed-off-by: Geoff Hutchison <[email protected]>
ghutchis · Oct 11, 2023 · a34b31c · a34b31c
1 parent 7f27522
commit a34b31c
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 7 deletions.
diff --git a/avogadro/io/fileformat.cpp b/avogadro/io/fileformat.cpp
@@ -5,6 +5,7 @@
 
 #include "fileformat.h"
 
+#include <algorithm>
 #include <fstream>
 #include <locale>
 #include <sstream>
@@ -15,22 +16,54 @@ using std::ifstream;
 using std::locale;
 using std::ofstream;
 
-FileFormat::FileFormat() : m_mode(None), m_in(nullptr), m_out(nullptr)
-{
-}
+FileFormat::FileFormat() : m_mode(None), m_in(nullptr), m_out(nullptr) {}
 
 FileFormat::~FileFormat()
 {
   delete m_in;
   delete m_out;
 }
 
+bool FileFormat::validateFileName(const std::string& fileName)
+{
+  bool valid = !fileName.empty();
+
+  if (valid) {
+    // check if the filename contains invalid characters
+    static std::string forbiddenChars(",^@={}[]~!?:&*\"|#%<>$\"'();`'");
+    valid = fileName.find_first_of(forbiddenChars) == std::string::npos;
+
+    // check if the filename contains ".." which we should not allow
+    valid = valid && fileName.find("..") == std::string::npos;
+  }
+
+  // Finally check against Windows names
+  // .. we do this on all platforms because CON.cif, for example
+  // is problematic to send to a Windows user.
+  if (valid) {
+    static std::string forbiddenNames(
+      "CON PRN AUX NUL COM1 COM2 COM3 COM4 COM5 "
+      "COM6 COM7 COM8 COM9 LPT1 LPT2 LPT3 LPT4 "
+      "LPT5 LPT6 LPT7 LPT8 LPT9");
+    // case insensitive search, since con.txt is also a problem
+    // https://stackoverflow.com/a/19839371/131896
+    auto it = std::search(fileName.begin(), fileName.end(),
+                          forbiddenNames.begin(), forbiddenNames.end(),
+                          [](unsigned char ch1, unsigned char ch2) {
+                            return std::toupper(ch1) == std::toupper(ch2);
+                          });
+    valid = (it == fileName.end());
+  }
+
+  return valid;
+}
+
 bool FileFormat::open(const std::string& fileName_, Operation mode_)
 {
   close();
   m_fileName = fileName_;
   m_mode = mode_;
-  if (!m_fileName.empty()) {
+  if (!m_fileName.empty() && validateFileName(m_fileName)) {
     // Imbue the standard C locale.
     locale cLocale("C");
     if (m_mode & Read) {
@@ -143,4 +176,4 @@ void FileFormat::appendError(const std::string& errorString, bool newLine)
     m_error += "\n";
 }
 
-} // namespace Avogadro
+} // namespace Avogadro::Io
diff --git a/avogadro/io/fileformat.h b/avogadro/io/fileformat.h
@@ -65,6 +65,18 @@ class AVOGADROIO_EXPORT FileFormat
    */
   virtual Operations supportedOperations() const = 0;
 
+  /**
+   * @brief Validates the given file name.
+   *
+   * Checks if the filename contains any invalid characters (e.g. ..)
+   * Also checks if the filename contains a restricted name on Windows.
+   * e.g., CON, PRN, AUX, NUL, COM1, COM2, COM3, COM4, COM5, etc.
+   *
+   * @param fileName The name of the file to be validated.
+   * @return true if the file name is valid, false otherwise.
+   */
+  static bool validateFileName(const std::string& fileName);
+
   /**
    * @brief Open the specified file in Read or Write mode.
    * @return True on success, false on failure.
@@ -252,7 +264,7 @@ inline FileFormat::Operation operator|(FileFormat::Operation a,
                                             static_cast<int>(b));
 }
 
-} // end Io namespace
-} // end Avogadro namespace
+} // namespace Io
+} // namespace Avogadro
 
 #endif // AVOGADRO_IO_FILEFORMAT_H
diff --git a/avogadro/qtgui/fileformatdialog.cpp b/avogadro/qtgui/fileformatdialog.cpp
@@ -42,6 +42,14 @@ FileFormatDialog::FormatFilePair FileFormatDialog::fileToRead(
     if (fileName.isEmpty()) // user cancel
       return result;
 
+    if (FileFormat::validateFileName(fileName.toStdString()) == false) {
+      QMessageBox::warning(
+        parent, caption,
+        tr("The file name contains invalid characters. Please choose another "
+           "file name."));
+      continue;
+    }
+
     const Io::FileFormat* format = findFileFormat(
       parent, caption, fileName, FileFormat::File | FileFormat::Read);