[PATCH] Fixed path traversal vulnerability.

Thanks to Sharon Brizinov >sharon.b@claroty.com> and Noam Moshe from
Claroty Research for the bug report and sample files.

This closes DCMTK issue #1021.

Gbp-Pq: Name f06a867513524664a1b03dfcf812d8b60fdd02cc.patch

diff --git a/dcmnet/apps/movescu.cc b/dcmnet/apps/movescu.cc
index 40f41674c0c443a8b5afc6f7c6e2d028047d81d9..7e444d46b2dac4d4d10f47e82a0a312114b2d07d 100644 (file)
--- a/dcmnet/apps/movescu.cc
+++ b/dcmnet/apps/movescu.cc
@@ -1,6 +1,6 @@
 /*
  *
- *  Copyright (C) 1994-2021, OFFIS e.V.
+ *  Copyright (C) 1994-2022, OFFIS e.V.
  *  All rights reserved.  See COPYRIGHT file for details.
  *
  *  This software and supporting documentation were developed by
@@ -1425,6 +1425,7 @@ static OFCondition storeSCP(
         sprintf(imageFileName, "%s.%s",
             dcmSOPClassUIDToModality(req->AffectedSOPClassUID),
             req->AffectedSOPInstanceUID);
+        OFStandard::sanitizeFilename(imageFileName);
     }
 
     OFString temp_str;

diff --git a/dcmnet/apps/storescp.cc b/dcmnet/apps/storescp.cc
index 434c406b24c9478a6e8d23d9098a88803a28ff86..ab1ef6befd8cb17395cfa740c6cb55d8deb00621 100644 (file)
--- a/dcmnet/apps/storescp.cc
+++ b/dcmnet/apps/storescp.cc
@@ -1853,12 +1853,14 @@ storeSCPCallback(
               if (!subdirectoryName.empty())
                 subdirectoryName += '_';
               subdirectoryName += currentStudyInstanceUID;
+              OFStandard::sanitizeFilename(subdirectoryName);
               break;
             case ESM_PatientName:
               // pattern: "[Patient's Name]_[YYYYMMDD]_[HHMMSSMMM]"
               subdirectoryName = currentPatientName;
               subdirectoryName += '_';
               subdirectoryName += timestamp;
+              OFStandard::sanitizeFilename(subdirectoryName);
               break;
             case ESM_None:
               break;
@@ -2065,9 +2067,11 @@ static OFCondition storeSCP(
     }
     else
     {
-      // don't create new UID, use the study instance UID as found in object
+      // Use the SOP instance UID as found in the C-STORE request message as part of the filename
+      OFString uid = req->AffectedSOPInstanceUID;
+      OFStandard::sanitizeFilename(uid);
       sprintf(imageFileName, "%s%c%s.%s%s", opt_outputDirectory.c_str(), PATH_SEPARATOR, dcmSOPClassUIDToModality(req->AffectedSOPClassUID, "UNKNOWN"),
-        req->AffectedSOPInstanceUID, opt_fileNameExtension.c_str());
+        uid.c_str(), opt_fileNameExtension.c_str());
     }
   }
 

diff --git a/dcmnet/libsrc/dstorscp.cc b/dcmnet/libsrc/dstorscp.cc
index e491ae5ea27d6e1e67be384e7a65828f321d6621..1811846a2a6cda8eac770f0c556b65f156233fc0 100644 (file)
--- a/dcmnet/libsrc/dstorscp.cc
+++ b/dcmnet/libsrc/dstorscp.cc
@@ -1,6 +1,6 @@
 /*
  *
- *  Copyright (C) 2013-2021, OFFIS e.V.
+ *  Copyright (C) 2013-2022, OFFIS e.V.
  *  All rights reserved.  See COPYRIGHT file for details.
  *
  *  This software and supporting documentation were developed by
@@ -425,6 +425,7 @@ OFCondition DcmStorageSCP::generateDirAndFilename(OFString &filename,
                     generatedFileName = tmpString;
                     OFSTRINGSTREAM_FREESTR(tmpString);
                     // combine the generated file name with the directory name
+                    OFStandard::sanitizeFilename(generatedFileName);
                     OFStandard::combineDirAndFilename(filename, directoryName, generatedFileName);
                 }
                 break;
@@ -441,6 +442,7 @@ OFCondition DcmStorageSCP::generateDirAndFilename(OFString &filename,
                 generatedFileName = tmpString;
                 OFSTRINGSTREAM_FREESTR(tmpString);
                 // combine the generated file name with the directory name
+                OFStandard::sanitizeFilename(generatedFileName);
                 OFStandard::combineDirAndFilename(filename, directoryName, generatedFileName);
                 break;
             }
@@ -469,6 +471,7 @@ OFCondition DcmStorageSCP::generateDirAndFilename(OFString &filename,
                     generatedFileName = tmpString;
                     OFSTRINGSTREAM_FREESTR(tmpString);
                     // combine the generated file name
+                    OFStandard::sanitizeFilename(generatedFileName);
                     OFStandard::combineDirAndFilename(filename, directoryName, generatedFileName);
                 } else
                     status = EC_CouldNotGenerateFilename;

diff --git a/dcmnet/libsrc/scu.cc b/dcmnet/libsrc/scu.cc
index 2a7c3eea12cbc99f6a435144dc402ed08714e202..f086778e25bf2fb20f20132d2ba5fbf5ceb2a712 100644 (file)
--- a/dcmnet/libsrc/scu.cc
+++ b/dcmnet/libsrc/scu.cc
@@ -1418,6 +1418,7 @@ OFString DcmSCU::createStorageFilename(DcmDataset* dataset)
     OFString name = dcmSOPClassUIDToModality(sopClassUID.c_str(), "UNKNOWN");
     name += ".";
     name += sopInstanceUID;
+    OFStandard::sanitizeFilename(name);
     OFString returnStr;
     OFStandard::combineDirAndFilename(returnStr, m_storageDir, name, OFTrue);
     return returnStr;

diff --git a/ofstd/include/dcmtk/ofstd/ofstd.h b/ofstd/include/dcmtk/ofstd/ofstd.h
index 1548e26d04b88bedde6cbab8018735e53cf6267f..56054ccb0c9010ccbe3cdbde233e5184bf0a7728 100644 (file)
--- a/ofstd/include/dcmtk/ofstd/ofstd.h
+++ b/ofstd/include/dcmtk/ofstd/ofstd.h
@@ -1,6 +1,6 @@
 /*
  *
- *  Copyright (C) 2000-2021, OFFIS e.V.
+ *  Copyright (C) 2000-2022, OFFIS e.V.
  *  All rights reserved.  See COPYRIGHT file for details.
  *
  *  This software and supporting documentation were developed by
@@ -1165,6 +1165,22 @@ class DCMTK_OFSTD_EXPORT OFStandard
     */
     static void forceSleep(Uint32 seconds);
 
+    /** sanitize a filename (NOT a path name!) by replacing all path
+     *  separators with underscores. This avoids possible path traversal
+     *  vulnerabilities if malformed data read from file or received over
+     *  a network is used as part of a filename.
+     *  @param fname filename to be sanitized
+     */
+    static void sanitizeFilename(OFString& fname);
+
+    /** sanitize a filename (NOT a path name!) by replacing all path
+     *  separators with underscores. This avoids possible path traversal
+     *  vulnerabilities if malformed data read from file or received over
+     *  a network is used as part of a filename.
+     *  @param fname filename to be sanitized
+     */
+    static void sanitizeFilename(char *fname);
+
  private:
 
     /** private implementation of strlcpy. Called when strlcpy

diff --git a/ofstd/libsrc/offname.cc b/ofstd/libsrc/offname.cc
index 832376189d14b1291f90f1471244328eb3480152..a56a7e1b586464da9986007ddd4a5ec2b2f6b9c7 100644 (file)
--- a/ofstd/libsrc/offname.cc
+++ b/ofstd/libsrc/offname.cc
@@ -1,6 +1,6 @@
 /*
  *
- *  Copyright (C) 1997-2021, OFFIS e.V.
+ *  Copyright (C) 1997-2022, OFFIS e.V.
  *  All rights reserved.  See COPYRIGHT file for details.
  *
  *  This software and supporting documentation were developed by
@@ -73,18 +73,23 @@ OFBool OFFilenameCreator::makeFilename(unsigned int &seed, const char *dir, cons
   {
     // create filename
     filename.clear();
-    if (dir)
-    {
-      filename = dir;
-      filename += PATH_SEPARATOR;
-    }
-    if (prefix) filename += prefix;
+    if (prefix) filename = prefix;
     addLongToString(creation_time, filename);
     // on some systems OFrand_r may produce only 16-bit random numbers.
     // To be on the safe side, we use two random numbers for the upper and the lower 16 bits.
     addLongToString((((OFrand_r(seed) & 0xFFFF) << 16) | (OFrand_r(seed) & 0xFFFF)), filename);
     if (postfix) filename += postfix;
 
+    OFStandard::sanitizeFilename(filename);
+
+    if (dir)
+    {
+      OFString dirname = dir;
+      dirname += PATH_SEPARATOR;
+      dirname += filename;
+      filename = dirname;
+    }
+
     // check if filename exists
     stat_result = stat(filename.c_str(), &stat_buf);
     if (stat_result == 0)

diff --git a/ofstd/libsrc/ofstd.cc b/ofstd/libsrc/ofstd.cc
index ae1466a9ff08dd4809b555005038e7e5155ed913..33ecd7954a76110bb1ab6d701f618dbf400a2637 100644 (file)
--- a/ofstd/libsrc/ofstd.cc
+++ b/ofstd/libsrc/ofstd.cc
@@ -1,6 +1,6 @@
 /*
  *
- *  Copyright (C) 2001-2021, OFFIS e.V.
+ *  Copyright (C) 2001-2022, OFFIS e.V.
  *  All rights reserved.  See COPYRIGHT file for details.
  *
  *  This software and supporting documentation were developed by
@@ -3245,6 +3245,39 @@ void OFStandard::forceSleep(Uint32 seconds)
     }
 }
 
+
+void OFStandard::sanitizeFilename(OFString& fname)
+{
+  size_t len = fname.length();
+  for (size_t i=0; i<len; ++i)
+  {
+#ifdef _WIN32
+        if ((fname[i] == PATH_SEPARATOR)||(fname[i] == '/')) fname[i] = '_';
+#else
+        if (fname[i] == PATH_SEPARATOR) fname[i] = '_';
+#endif
+  }
+}
+
+
+void OFStandard::sanitizeFilename(char *fname)
+{
+  if (fname)
+  {
+    char *c = fname;
+    while (*c)
+    {
+#ifdef _WIN32
+      if ((*c == PATH_SEPARATOR)||(*c == '/')) *c = '_';
+#else
+      if (*c == PATH_SEPARATOR) *c = '_';
+#endif
+      ++c;
+    }
+  }
+}
+
+
 #include DCMTK_DIAGNOSTIC_IGNORE_STRICT_ALIASING_WARNING
 
 // black magic: