Sanitize all strings passed to the exec options.

Applied-Upstream: edbb085e45788dccaf0e64d71534cfca925784b8
Last-Update: 2026-03-21
Bug: https://support.dcmtk.org/redmine/issues/1194
Bug-Debian: https://bugs.debian.org/1133001
Reviewed-By: Étienne Mollier <emollier@debian.org>
Sanitize the text fields from incoming DICOM associations and DICOM objects
(such as Study Instance UID, SOP Instance UID, Patient's Name) and the
calling SCU's network presentation address by removing special characters
that may be interpreted as shell escape characters when one of the
execution options (e.g. --exec-on-reception) is in use.

Thanks to Machine Spirits UG (haftungsbeschränkt) for the bug report,
detailed analysis and proof of concept.

This closes DCMTK issue #1194.

Gbp-Pq: Name CVE-2026-5663.patch

diff --git a/dcmnet/apps/storescp.cc b/dcmnet/apps/storescp.cc
index 7a69a16144171fa551de89c758c8ed79631c3df8..183c92f1e059656e79f5b8da54707bf3c18acbe0 100644 (file)
--- a/dcmnet/apps/storescp.cc
+++ b/dcmnet/apps/storescp.cc
@@ -1,6 +1,6 @@
 /*
  *
- *  Copyright (C) 1994-2025, OFFIS e.V.
+ *  Copyright (C) 1994-2026, OFFIS e.V.
  *  All rights reserved.  See COPYRIGHT file for details.
  *
  *  This software and supporting documentation were developed by
@@ -1601,7 +1601,9 @@ static OFCondition acceptAssociation(T_ASC_Network *net, DcmAssociationConfigura
     calledAETitle.clear();
   }
   // store calling presentation address (i.e. remote hostname)
-  callingPresentationAddress = OFSTRING_GUARD(assoc->params->DULparams.callingPresentationAddress);
+  callingPresentationAddress = "\"";
+  callingPresentationAddress += OFSTRING_GUARD(assoc->params->DULparams.callingPresentationAddress);
+  callingPresentationAddress += "\"";
 
   /* now do the real work, i.e. receive DIMSE commands over the network connection */
   /* which was established and handle these commands correspondingly. In case of */
@@ -1965,6 +1967,7 @@ storeSCPCallback(
             dateTime.getTime().getHour(), dateTime.getTime().getMinute(), dateTime.getTime().getIntSecond(), dateTime.getTime().getMilliSecond());
 
           OFString subdirectoryName;
+          OFString s;
           switch (opt_sortStudyMode)
           {
             case ESM_Timestamp:
@@ -1979,15 +1982,27 @@ storeSCPCallback(
               subdirectoryName = opt_sortStudyDirPrefix;
               if (!subdirectoryName.empty())
                 subdirectoryName += '_';
-              subdirectoryName += currentStudyInstanceUID;
-              OFStandard::sanitizeFilename(subdirectoryName);
+              s = currentStudyInstanceUID;
+              OFStandard::sanitizeFilename(s);
+              if (s != currentStudyInstanceUID)
+              {
+                OFLOG_WARN(storescpLogger, "Sanitized unusual characters in Study Instance UID, converted from \"" << currentStudyInstanceUID << "\" to \"" << s << "\".");
+              }
+              subdirectoryName += s;
               break;
             case ESM_PatientName:
               // pattern: "[Patient's Name]_[YYYYMMDD]_[HHMMSSMMM]"
               subdirectoryName = currentPatientName;
+              OFStandard::sanitizeFilename(subdirectoryName);
+              if (subdirectoryName != currentPatientName)
+              {
+                // It is quite normal that we need to sanitize characters in PatientName.
+                // Therefore, this is only a debug message and not a warning, unlike the other
+                // messages about sanitized fields, which are normally not expected.
+                OFLOG_DEBUG(storescpLogger, "Sanitized characters in Patient Name, converted from \"" << currentPatientName << "\" to \"" << subdirectoryName << "\".");
+              }
               subdirectoryName += '_';
               subdirectoryName += timestamp;
-              OFStandard::sanitizeFilename(subdirectoryName);
               break;
             case ESM_None:
               break;
@@ -2196,8 +2211,13 @@ static OFCondition storeSCP(
     else
     {
       // Use the SOP instance UID as found in the C-STORE request message as part of the filename
-      OFString uid(OFSTRING_GUARD(req->AffectedSOPInstanceUID));
+      OFString s(OFSTRING_GUARD(req->AffectedSOPInstanceUID));
+      OFString uid = s;
       OFStandard::sanitizeFilename(uid);
+      if (uid != s)
+      {
+        OFLOG_WARN(storescpLogger, "Sanitized unusual characters in SOP Instance UID, converted from \"" << s << "\" to \"" << uid << "\".");
+      }
       OFStandard::snprintf(imageFileName, sizeof(imageFileName), "%s%c%s.%s%s", opt_outputDirectory.c_str(), PATH_SEPARATOR, dcmSOPClassUIDToModality(req->AffectedSOPClassUID, "UNKNOWN"),
         uid.c_str(), opt_fileNameExtension.c_str());
     }
@@ -2367,16 +2387,19 @@ static void executeOnReception()
   if( !opt_ignore )
   {
     // perform substitution for placeholder #p (depending on presence of any --sort-xxx option)
+    // Note: We do not enclose this in quotes because it may be used as part of a path expression.
     OFString dir = (opt_sortStudyMode == ESM_None) ? opt_outputDirectory : subdirectoryPathAndName;
     cmd = replaceChars( cmd, OFString(PATH_PLACEHOLDER), dir );
 
     // perform substitution for placeholder #f; note that outputFileNameArray.back()
     // always contains the name of the file (without path) which was written last.
+    // Note: We do not enclose this in quotes because it may be used as part of a path expression.
     OFString outputFileName = outputFileNameArray.back();
     cmd = replaceChars( cmd, OFString(FILENAME_PLACEHOLDER), outputFileName );
   }
 
-  // perform substitution for placeholder #a
+  // perform substitution for placeholder #a.
+  // Note that this string is already enclosed in double quotes at this point
   s = callingAETitle;
   sanitizeAETitle(s);
   if (s != callingAETitle)
@@ -2385,7 +2408,8 @@ static void executeOnReception()
   }
   cmd = replaceChars( cmd, OFString(CALLING_AETITLE_PLACEHOLDER), s );
 
-  // perform substitution for placeholder #c
+  // perform substitution for placeholder #c.
+  // Note that this string is already enclosed in double quotes at this point
   s = calledAETitle;
   sanitizeAETitle(s);
   if (s != calledAETitle)
@@ -2394,8 +2418,15 @@ static void executeOnReception()
   }
   cmd = replaceChars( cmd, OFString(CALLED_AETITLE_PLACEHOLDER), s );
 
-  // perform substitution for placeholder #r
-  cmd = replaceChars( cmd, OFString(CALLING_PRESENTATION_ADDRESS_PLACEHOLDER), callingPresentationAddress );
+  // perform substitution for placeholder #r.
+  // Note that this string is already enclosed in double quotes at this point
+  s = callingPresentationAddress;
+  sanitizeAETitle(s);
+  if (s != callingPresentationAddress)
+  {
+    OFLOG_WARN(storescpLogger, "Sanitized unusual characters in calling presentation address, converted from " << callingPresentationAddress << " to " << s << ".");
+  }
+  cmd = replaceChars( cmd, OFString(CALLING_PRESENTATION_ADDRESS_PLACEHOLDER), s );
 
   // Execute command in a new process
   executeCommand( cmd );
@@ -2500,20 +2531,38 @@ static void executeOnEndOfStudy()
   OFString s;
 
   // perform substitution for placeholder #p; #p will be substituted by lastStudySubdirectoryPathAndName
+  // Note: We do not enclose this in quotes because it may be used as part of a path expression.
   cmd = replaceChars( cmd, OFString(PATH_PLACEHOLDER), lastStudySubdirectoryPathAndName );
 
-  // perform substitution for placeholder #a
+  // perform substitution for placeholder #a.
+  // Note that this string is already enclosed in double quotes at this point
   s = callingAETitle;
   sanitizeAETitle(s);
+  if (s != callingAETitle)
+  {
+    OFLOG_WARN(storescpLogger, "Sanitized unusual characters in calling aetitle, converted from " << callingAETitle << " to " << s << ".");
+  }
   cmd = replaceChars( cmd, OFString(CALLING_AETITLE_PLACEHOLDER), s );
 
-  // perform substitution for placeholder #c
+  // perform substitution for placeholder #c.
+  // Note that this string is already enclosed in double quotes at this point
   s = calledAETitle;
   sanitizeAETitle(s);
+  if (s != calledAETitle)
+  {
+    OFLOG_WARN(storescpLogger, "Sanitized unusual characters in called aetitle, converted from " << calledAETitle << " to " << s << ".");
+  }
   cmd = replaceChars( cmd, OFString(CALLED_AETITLE_PLACEHOLDER), s );
 
-  // perform substitution for placeholder #r
-  cmd = replaceChars( cmd, OFString(CALLING_PRESENTATION_ADDRESS_PLACEHOLDER), callingPresentationAddress );
+  // perform substitution for placeholder #r.
+  // Note that this string is already enclosed in double quotes at this point
+  s = callingPresentationAddress;
+  sanitizeAETitle(s);
+  if (s != callingPresentationAddress)
+  {
+    OFLOG_WARN(storescpLogger, "Sanitized unusual characters in calling presentation address, converted from " << callingPresentationAddress << " to " << s << ".");
+  }
+  cmd = replaceChars( cmd, OFString(CALLING_PRESENTATION_ADDRESS_PLACEHOLDER), s );
 
   // Execute command in a new process
   executeCommand( cmd );

diff --git a/ofstd/libsrc/ofstd.cc b/ofstd/libsrc/ofstd.cc
index 3524695ae812ace22f147dd87228a379d51fa0e3..78284b049aa7987ddcd731be0823cecc1b7b1744 100644 (file)
--- a/ofstd/libsrc/ofstd.cc
+++ b/ofstd/libsrc/ofstd.cc
@@ -3402,16 +3402,26 @@ void OFStandard::forceSleep(Uint32 seconds)
 }
 
 
+static const char sanitized_filename_charset[] =
+{
+  ' ', '_', '_', '_', '_', '_', '_', '_', '_', '_', '_', '_', '_', '-', '.', '_',
+  '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', ':', '_', '_', '_', '_', '_',
+  '@', 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O',
+  'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', '_', '_', '_', '_', '_',
+  '_', 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o',
+  'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', '_', '_', '_', '_', '_'
+};
+
+
 void OFStandard::sanitizeFilename(OFString& fname)
 {
     const size_t len = fname.length();
+    char c;
     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
+        c = fname[i];
+        if (c != 0 && (c < 32 || c >= 127)) c = '_'; else c = sanitized_filename_charset[c-32];
+        fname[i] = c;
     }
 }
 
@@ -3423,11 +3433,7 @@ void OFStandard::sanitizeFilename(char *fname)
         char *c = fname;
         while (*c)
         {
-#ifdef _WIN32
-            if ((*c == PATH_SEPARATOR) || (*c == '/')) *c = '_';
-#else
-            if (*c == PATH_SEPARATOR) *c = '_';
-#endif
+            if (*c < 32 || *c >= 127) *c = '_'; else *c = sanitized_filename_charset[*c-32];
             ++c;
         }
     }